-
Notifications
You must be signed in to change notification settings - Fork 24
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
Reset inputs after exported for out_grad #238
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
onnx_chainer/export.py
Outdated
@@ -407,6 +413,8 @@ def _export(model, args, filename, export_params, graph_name, save_text, | |||
o = Graph(context, converters, opset_version, network_outputs) | |||
o.to_onnx_graph() | |||
|
|||
hook.restore_inputs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[optional] I think it'd be better to use __exit__
and make the scope of hook
larger since, ideally, this should be done even when an exception is thrown. I don't think we should do this in this PR, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, fixed
Codecov Report
@@ Coverage Diff @@
## master #238 +/- ##
==========================================
+ Coverage 91.37% 91.39% +0.02%
==========================================
Files 24 24
Lines 1611 1615 +4
==========================================
+ Hits 1472 1476 +4
Misses 139 139
Continue to review full report at Codecov.
|
/test |
Successfully created a job for commit 97c13dc: |
train=False
,output_grad=True
then got
This PR fixed unexepected retained variable node by restored hooked inputs.