Skip to content
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

*rule_test should take a frule_like/rrule_like function so we can test AD systems #114

Closed
oxinabox opened this issue Feb 3, 2021 · 3 comments · Fixed by #166
Closed
Milestone

Comments

@oxinabox
Copy link
Member

oxinabox commented Feb 3, 2021

In order to make frule_test and rrule_test more generally useful for testing AD systems,
we should make them take a keyword argument that specifies what function that what to use.
This function would be used in the place of the default frule/rrule.

It would need to match the API of frule and rrule.
Just like the function that needs to be provided for JuliaDiff/ChainRulesCore.jl#68

Then we would be able to use these tools to test AD systems

@willtebbutt
Copy link
Member

@oxinabox do you recall if this is included in ADIA? @matbesancon also requested it in #125 , and I keep realising that I need it when I want to check that AD is doing the right thing for a function that I particularly care about, even if I've not written any new rules.

@mzgubic
Copy link
Member

mzgubic commented Mar 12, 2021

I don't think it is, but we can probably add it - I have only considered this superficially but it doesn't seem like a huge task?

@willtebbutt
Copy link
Member

willtebbutt commented Mar 12, 2021

Yeah, it really shouldn't be too large a task.

My opinion is that we should limit the scope such that we require whatever function is passed to have the same API as frule/ rrule, and its the responsibility of whoever is calling the tests to ensure that this is satisfied. I think that would already put us in a pretty good place.

If we do that, it ought to be a very quick job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants