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
Move constants in InferenceParameters into ParameterDict #145
Conversation
@meissnereric could you look into the reason that the test cases fail? |
Yeah sure, they passed locally for me. |
Appears to be an issue in how MXNet differs in Python 3.4/5 from 3.6 which changes the gradient calculations of Constants. This is probably why only the ScoreFunction tests break since they're the only ones where we numerically check gradients. I can replicate it by running the tests in Python 3.4/5, and reverting the Constant changes make the tests pass again. |
Could we identify which API of mxnet changes behaviors between 3.4/3.5 and 3.6? |
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 am not sure whether we want to go ahead with the changes about constant, but I like the documentation about inference. We should definitely merge the documentation part.
Co-Authored-By: meissnereric <meissner.eric.7@gmail.com>
Description of changes:
Shift MXNet based constants from being stored in the InferenceParameters._constants dictionary (which still exists and keeps scalar/native constants) to keeping MXNet constants in the ParameterDict where other Parameters are kept.
This removes the need for the separate MXNet constants file at serialization file as well!
(Feel free to ignore the documentation change here, it's a remnant of the larger docs change I made in another PR and I'll try to make sure the other one is the one that gets kept.)
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.