Skip to content
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

Multiple fixes for Bert Model (fine-tuned) #929

Merged
merged 6 commits into from May 20, 2020

Conversation

jignparm
Copy link
Contributor

Allow for dynamic output shapes in Onnx model [ i.e. rank=1, shape =(?,)]
Fix 'Squeeze' op to handle shape of (?,)
Refactor 'Constant_folding' rewriter to pass unit tests

Allow output shape of (?,)
Fix Squeeze op when tensor shape is (?,)
Copy link
Collaborator

@guschmue guschmue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

try:
inputs = []
skip = False
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need the skip? we already check that all inputs are const below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the catch! inputs and len(op.input) == len(inputs) will only run if all are const. Removed the skip

@@ -466,7 +460,7 @@ def compat_handler(ctx, node, **kwargs):
rewrite_single_direction_lstm, rewrite_bi_direction_lstm,
rewrite_single_direction_gru, rewrite_bi_direction_gru,
rewrite_custom_rnn_cell, rewrite_generic_loop, rewrite_cond,
rewrite_biasadd_with_conv2d,
rewrite_biasadd_with_conv2d, rewrite_constant_fold
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might help to run const_cold first in the longer term but maybe more risk right now

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it to first rewriter.

@jignparm jignparm merged commit f333ce5 into onnx:master May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants