-
Notifications
You must be signed in to change notification settings - Fork 130
Implement scalar loop for iterative gradients #283
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
Conversation
1a36f5c
to
2d36eb1
Compare
2d36eb1
to
6ac04d7
Compare
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've never tried something like this before so I hope it can be a bit useful. The test coverage for some of the errors in loop.py
is spotty, but I don't know if this matters. There's also no test for half precision computation.
@aseyboldt mentioned we should be able to implement the gradient as another ScalarLoop, via forward auto-diff, which in this case should be equivalent to backward-autodiff. It should also be possible to fuse the loops, but hopefully that's one thing the compilers can easily figure out, once they get merged inside the same Composite? |
660ab98
to
6ce2143
Compare
We have a job that runs on float32 by default if that's what you meant? |
# TODO: We could convert to TensorVariable, optimize graph, | ||
# and then convert back to ScalarVariable. | ||
# This would introduce rewrites like `log(1 + x) -> log1p`. |
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.
Wouldn't it be more useful to make the rewrites also apply to scalar variables?
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.
That would be nice, but there is no automatic way of doing that. Almost all of our rewrites are written with TensorVariables in mind and they would probably error out quickly if you passed ScalarVariables as inputs
# _c_code += 'printf("inputs=[");' | ||
# for i in range(1, len(fgraph.inputs)): | ||
# _c_code += f'printf("%%.16g, ", %(i{i})s);' | ||
# _c_code += 'printf("]\\n");\n' |
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.
what's this about?
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.
This was code I used for debug that I imagine would be very helpful for someone trying to debug the c implementation in the future. Same for the other commented code I left
# _c_code += 'printf("%%ld\\n", i);\n' | ||
# for carry in range(1, 10): | ||
# _c_code += f'printf("\\t %%.g\\n", i, %(i{carry})s_copy{carry-1});\n' |
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.
here too
6ce2143
to
34230a2
Compare
Used in the derivatives of
betainc
,gammainc
,gammaincc
andhyp2f1
.It should be considerably faster than the old numpy loops, and easier to dispatch to numba and jax as well.
Alternative to #174
Closes #83
This is the kind of C code that is generated for the gammaincc gradient inner loop:
Toggle C code