Bug fixes: nir.Output type inference & CubaLIF.w_in read/write #71
Conversation
Right now, the type inference goes through but it doesn't actually change the NIR graph. This PR fixes that.
|
|
|
yep I just wrote tests to check that this works, but the problem was something else. I will update the PR in a moment - sorry about the confusion |
stevenabreu7
changed the title
stevenabreu7
changed the title
| @@ -261,6 +261,12 @@ def _forward_type_inference(self, debug=True): | |||
| k.replace('output', 'input'): v for k, v in pre_node.output_type.items() | |||
| } | |||
|
|
|||
| # make sure that output nodes have output_type = input_type | |||
| if isinstance(post_node, Output): | |||
| post_node.output_type = { | |||
There was a problem hiding this comment.
Hmmmm this looks like a hack
There was a problem hiding this comment.
it feels hacky, but the output node is the only node whose input_type field is irrelevant - so it's just a special case where we just have to set the output_type as well. a cleaner solution would be that Input and Output only have a type - but that would complicate many other things..
| @@ -72,6 +72,7 @@ def read_node(node: typing.Any) -> nir.NIRNode: | |||
| r=node["r"][()], | |||
| v_leak=node["v_leak"][()], | |||
| v_threshold=node["v_threshold"][()], | |||
| w_in=node["w_in"][()], | |||
Right now, the type inference goes through but it doesn't actually change the NIR graph. This PR fixes that.Update: this PR fixes a weird edge case where the input_type of a
nir.Outputnode is inferred. This currently fails because it will not update the output_type for thenir.Outputnode and this is used when writing the graph to a file. (hence, when writing to a file and reading again, the shape inference is essentially discarded).Update update: I've also added an urgent fix that adds
w_into the read and write method of theCubaLIFnode. This should have always been there..