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

frule invalidation broken #269

Open
oxinabox opened this issue Feb 13, 2024 · 4 comments
Open

frule invalidation broken #269

oxinabox opened this issue Feb 13, 2024 · 4 comments
Labels
bug Something isn't working forward-mode

Comments

@oxinabox
Copy link
Member

We actually have a test for this: https://github.com/JuliaDiff/Diffractor.jl/blob/b37f585d4ba8fa5cb607cd75aaabb5464eef6633/test/rules.jl
but it turns out this file has never been linked up to the test/runtests.jl file.
So it wasn't getting run by CI.

The last test in that file fails.
Both on 1.10 and on nightly

@oxinabox oxinabox added bug Something isn't working forward-mode labels Feb 13, 2024
@oxinabox
Copy link
Member Author

We do manually insert an edge to the MethodInstance being AD'ed here

ci.edges = MethodInstance[mi]

Do we need to manually insert an edge to every method we call in the generated code?
Surely not, we haven't run type-inference so we can't even know that, and that surely already will insert its own edges, right?

@Keno
Copy link
Collaborator

Keno commented Feb 13, 2024

No, but we might need an edge for the frule of the MethodInstance we're currently ADing to cover the case where an frule is added to something that had none before.

@oxinabox
Copy link
Member Author

I don't think so.
That case would get caught from the calling code, no?
Since ∂☆ always calls ∂☆internal first
and that does (basically):

function ∂☆internal(...)
    res = frule(...)
    if nothing===res
        ∂☆recurse...  # this code we are talking about
    else
        shuffle_base(res)
    end
end

So adding a new frule should already invalidate it, since it invalidates the top line of ∂☆internal

@Keno
Copy link
Collaborator

Keno commented Feb 13, 2024

Ah yes, probably. Something else going on then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working forward-mode
Projects
None yet
Development

No branches or pull requests

2 participants