-
Notifications
You must be signed in to change notification settings - Fork 95
Overhaul Rules (partner PR) #91
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
cab0e9e to
f564800
Compare
| end | ||
| end | ||
|
|
||
| function ensure_not_running_on_functor(f, name) |
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 is temporary and will change in future? let's comment that?
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.
Not super soon,
@test_scalar would need a lot of work to handle functors.
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.
yeah, i'm fine with that... maybe we should have an issue tracking that and link to it here?
it certainly is on the long-term plan (although someone might need to want it enough for it to actually happen)
|
Status update:
|
|
Everything is done accept sorting out things that accumulate inplace. |
Scalar functions changed over
Temp comment out all accumulate related tests
f002d6e to
aafadfa
Compare
nickrobinson251
left a comment
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.
looking good
since there are so many changes I'll try to review again - probably once the "un-named returns" stuff is fixed up since I think that the last notable thing left?
Co-Authored-By: Nick Robinson <[email protected]>
Co-Authored-By: Nick Robinson <[email protected]>
Co-Authored-By: Nick Robinson <[email protected]>
|
This should be good to go now |
nickrobinson251
left a comment
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 have reviewed the source
feels good
mostly some tiny style suggestions
will review tests later today
Co-Authored-By: Nick Robinson <[email protected]>
nickrobinson251
left a comment
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.
Let's do this
Co-Authored-By: Nick Robinson <[email protected]>
This is the matching changes for
JuliaDiff/ChainRulesCore.jl#30
Before reviewing you are recommended to read the Docs PR buddy of this:
#103