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
Add get_input() to DLR #44
Conversation
@hcho3 code looks good! Please add a unit test. |
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.
Could you please explain a bit why GetInput
is needed? Shouldn't users already know the input?
Still need to test a few things. I'll take off "Draft" status once this PR is ready. |
@zhiics No, since the value of the input changes when the cc @kevinthesun |
@hcho3 I see. Thanks. |
The current logic for getting input names assumes that inputs always precede parameters, but it is not always true. One model I am looking at actually has some parameters before the input:
EDIT. Fixed by commit fb87ad4 |
Also, Lines 196 to 203 in 0ce46d9
EDIT. Fixed by commit fb87ad4 |
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
We need the ability to get the current value of an input when using the
_assign
operator in NNVM.