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

Precompilation fails while using ReverseDiff backend #1400

Closed
BlackWingedKing opened this issue Sep 6, 2020 · 9 comments
Closed

Precompilation fails while using ReverseDiff backend #1400

BlackWingedKing opened this issue Sep 6, 2020 · 9 comments

Comments

@BlackWingedKing
Copy link

I have created an MWE here. The description is in README.md of the repository.
This reply in the slack thread might be relevant

@BlackWingedKing BlackWingedKing changed the title Precompilation fails with using ReverseDiff backend Precompilation fails while using ReverseDiff backend Sep 6, 2020
@devmotion
Copy link
Member

As explained on Slack, Turing.setadbackend(:reverse_diff) is incorrect and should not be used - it does not work even if you run it in a script.

On the other hand Turing.setadbackend(:reversediff) (or Turing.setadbackend(Val(:reversediff))) which should be used is only defined inside of a @require block if ReverseDiff is loaded. Reading through JuliaLang/Pkg.jl#1285 (comment), it seems that one of the bad things about optional dependencies with Requires is that the optionally loaded code does not end up in the precompile files. I suspect this fundamental problem with Requires leads to the precompilation issues - however, a MWE without Turing would be needed to confirm this hypothesis.

@BlackWingedKing
Copy link
Author

BlackWingedKing commented Sep 6, 2020

Hi @devmotion
Thank you for your reply. I understand the issue. I have created this fork which merges this PR with master and adds a few things on top of it.

  • Add ReverseDiff and Memoization as a dependency
  • Remove the @requires macro for both ReverseDiff and Memoization

But I still get a similar error with :reversediff backend
This README provides steps for reproducing the over the MWE

@devmotion
Copy link
Member

Ah nice, so it seems my hypothesis was correct and the precompilation problem is caused by Requires. The error messages that you get now with your Turing fork without Requires are caused by AdvancedVI - and there exactly the same problem exists: the ReverseDiff support is optional, hidden in a @require block. I assume if you would make ReverseDiff a proper dependency of both AdvancedVI and Bijectors (the same problem should be present there as well) as well, then precompilation should work.

@BlackWingedKing
Copy link
Author

BlackWingedKing commented Sep 6, 2020

Thank you @devmotion. I will make the corresponding changes.
Also, do you people have any plans to add ReverseDIff and Memoization as a dependency for Turing?
Or is it possible to have ReverseDiffRules.jl pkg similar to ZygoteRules?

@devmotion
Copy link
Member

Or is it possible to have ReverseDiffRules.jl pkg similar to ZygoteRules?

ReverseDiff will support ChainRules (but it is not clear yet when) (which BTW should also be used to implement custom adjoints for Zygote - we need ZygoteRules though since we want to get the Zygote-specific pullback here). Maybe together with something like #1402 that would allow use to remove any optional dependencies for AD.

It seems ReverseDiff and in particular Memoization wouldn't pull in too many additional dependencies though. So the main question would be how much these additional dependencies would increase loading times I guess.

@trappmartin
Copy link
Member

Can this be closed?

@BlackWingedKing
Copy link
Author

yes @trappmartin I think this can be closed. If anyone wants to precompile with :reversediff backend they would need to remove @requires block for ReverseDiff, Memoization and add them as a dependency to Turing.jl and AdvancedVI.jl
I think it's better if this is mentioned somewhere.

@BlackWingedKing
Copy link
Author

BlackWingedKing commented Sep 30, 2020

@trappmartin can this be added to Autodiff page as a note at the end of switch-ad modes block?

custom packages using reversediff with rdcache won't be precompiled because Reversediff and Memoization are an optional dependency to Turing.jl and AdvancedVI.jl refer issue #1400 of Turing and JuliaLang/Pkg.jl#1285 (comment)

@yebai
Copy link
Member

yebai commented Nov 12, 2022

Likely fixed by #1877

@yebai yebai closed this as completed Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants