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

[docs] document how to support operators with several vector arguments #3577

Merged
merged 2 commits into from
Nov 22, 2023

Conversation

odow
Copy link
Member

@odow odow commented Nov 20, 2023

Closes #3576

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (283a99d) 98.38% compared to head (65ac069) 98.38%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3577   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files          37       38    +1     
  Lines        5629     5648   +19     
=======================================
+ Hits         5538     5557   +19     
  Misses         91       91           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@odow
Copy link
Member Author

odow commented Nov 20, 2023

@blegat
Copy link
Member

blegat commented Nov 21, 2023

It might be more helpful to show how to deal with functions that cannot be traced and to show how to also get the derivative info with AD and pass it to @operator

@odow
Copy link
Member Author

odow commented Nov 21, 2023

@blegat
Copy link
Member

blegat commented Nov 21, 2023

It doesn't say that you can use AD

@odow
Copy link
Member Author

odow commented Nov 21, 2023

I'd rather not encourage people. We can already use ForwardDiff automatically. If anyone has a user-defined operator and wants to use a custom AD that isn't ForwardDiff, then I expect them to be very advanced users who can figure it out based on the signature of grad_f(g::AbstractVector{T}, x::T...) where {T}.

If we tell people to use Enzyme/Zygote/ReverseDiff.jl then we'll have to deal with the questions and quirks.

@odow
Copy link
Member Author

odow commented Nov 22, 2023

This is good enough for now. A question of whether to add a third-party AD example is best left for a different PR. I'll make a note in the issue.

@odow odow merged commit ca42d00 into master Nov 22, 2023
12 checks passed
@odow odow deleted the od/doc-vector-splat branch November 22, 2023 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Operator with vector arguments
2 participants