Fix Shape, Reshape, and invalid rust variable names.

[Knight-Ops]

Pull Request Template

Checklist

• Confirmed that run-checks all script has been executed.
• Made sure the book is up to date with changes in this PR.

Related Issues/PRs

#2930

Changes

Shape

• shape_update_outputs : Changed the output type to a Tensor as per the Onnx specification. It always outputs a Tensor, not a Shape.

Reshape

• Added improved error reporting. Its really hard to tell where errors are coming from when doing the build, so I made sure everything had Reshape: for some extra information
• reshape_config : Removed check on input length and input value. The input.value is None, because the input is a Tensor, you can't use value you have to check the Tensor.shape()
• shape_update_outputs : Fixed output Tensor elem_type and ensured that shape was a valid Vec, this is again done by not trying to get the value, but instead looking at the shape on the Tensor.

format_name

• Simple change, just replace characters that are invalid Rust variable names with underscores. This could possibly be done with a regex, but I don't know that it is entirely necessary right now. I just replaced the characters I ran into issues with while testing.

Testing

Run cargo test on burn-import crate with all passing. Also using file referenced in #2930 as a test file.

[Knight-Ops]

Not sure what really to add here, I added back in some of the old code as a first approach specific on Reshape the model referenced in #2930 didn't like how Reshape was implemented (Value never existed). But I have a TF exported model that had a regression on it due to these changes, I believe it may be due to constant folding for the Reshape, but I don't have 100% confirmation on that.

My best guess here is that the original code worked for Constant shapes, where-as in theory you can have a Tensor calculated at runtime based on my understanding. We should check first for constants and then fall back to dynamic tensors.

[laggui]

Just a comment regarding the ArgType::Shape.

This is part of the intermediate representation between ONNX <> Burn. In burn shapes are not actually tensors, but arrays. I believe this was the initial motivation behind the variants for ArgType.

ONNX uses tensor types for pretty much all values, but the reality is a bit more complex. Even scalars are simply represented as 0-dim tensors.

So the output of shape should remain ArgType::Shape to maintain compatibility with the current codegen.

I haven't looked at the underlying cause in the linked issue, but from the error it looks like there is a node that should handle the ArgType::Shape.