-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Deps] Update qonnx to track https://github.com/iksnagreb/qonnx #9
Conversation
I get the following for a Transformer build that previously worked on iksnagreb/qonnx/transformer (+ 2 unrelated commits cherry-picked from main):
|
Hm, as the ScaledIntType issue seems to be fixed on some of the range analysis related branches and I will need this stuff for ongoing/upcoming research anyway, I am seriously considering to merge the current state of range analysis into our qonnx dependency. The experimental analysis pass and the (not yet fully integrated) reworked streamlining should not interfere with any of our build flows (these are just optional extras for now), but somewhere among the ~130 commits must be the bugfix for whatever this ScaledIntType issue is... What do you think? |
I still get the ScaledIntType error if I switch to this QONNX commit: iksnagreb/qonnx@31880be This is the old commit where synthesis and verification still passes: iksnagreb/qonnx@1888fab |
Seems to fix some issues related to scaled-int types and brings in infrastructure required by ongoing/upcoming research.
Could someone (maybe @fpjentzsch, you have the infrastructure for testing) do some non-trivial (i.e., not just some one layer MLP...) build with opset version 12 (or maybe even higher, to see how far we could push this)? Then we could decide to merge or close the one remaining PR here... |
We can run the test suite afterwards in combination with the other Transformer-related PRs. Feel free to create a separate PR for the opset version 12 if needed. |
Discuss whether to include the following PRs here as well...