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

Yota support #50

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Yota support #50

wants to merge 6 commits into from

Conversation

mohamed82008
Copy link
Member

This PR is based on #49 but tests here fail. I am opening it in case @dfdx or someone else can figure out why they fail when Zygote passes.

@dfdx
Copy link

dfdx commented Feb 8, 2022

I see one error coming from a Yota bug that is already fixed on main, but I can't immediately understand the others, especially related to the arity of functions, e.g.:

MethodError: no method matching fder(::Float64)
Closest candidates are:
fder(::Any, ::Any) at /home/runner/work/AbstractDifferentiation.jl/AbstractDifferentiation.jl/test/test_utils.jl:6

Do you have a minimal code example that reproduces them? If not, give me a couple of days to learn internals of AbstractDifferentiation and find where the problem comes from.

@codecov-commenter
Copy link

Codecov Report

Merging #50 (89ed1b8) into master (8f0d6db) will decrease coverage by 5.15%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #50      +/-   ##
==========================================
- Coverage   83.36%   78.21%   -5.16%     
==========================================
  Files           6        6              
  Lines         475      358     -117     
==========================================
- Hits          396      280     -116     
+ Misses         79       78       -1     
Impacted Files Coverage Δ
src/AbstractDifferentiation.jl 77.41% <0.00%> (-1.00%) ⬇️
src/ruleconfig.jl 0.00% <0.00%> (-100.00%) ⬇️
src/finitedifferences.jl 75.00% <0.00%> (-25.00%) ⬇️
src/tracker.jl 83.33% <0.00%> (-16.67%) ⬇️
src/forwarddiff.jl 86.20% <0.00%> (-13.80%) ⬇️
src/reversediff.jl 92.00% <0.00%> (-8.00%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f0d6db...89ed1b8. Read the comment docs.

@mohamed82008
Copy link
Member Author

mohamed82008 commented Feb 9, 2022

Do you have a minimal code example that reproduces them?

Unfortunately no. I can dig into it next week if you don't beat me to it (please do :)).

@dfdx
Copy link

dfdx commented Feb 22, 2022

I managed to fix a few cases, but the amount of errors only grows, and I don't understand the code base here well enough to understand what exactly is expected. Is there a minimal set of interfaces that need to be implemented? Perhaps, value_and_xxx set of methods?

@mohamed82008
Copy link
Member Author

mohamed82008 commented Feb 23, 2022

Some of the internals here will be redesigned soon so I would wait for that to happen before trying to understand them :)

@gdalle gdalle marked this pull request as draft September 20, 2023 11:12
@gdalle gdalle added the feature New feature or request label Oct 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants