Use Base.Fix1 instead of _logdensity_closure
#86
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks! Could you tag a new release such that I can use the changes in the Turing PR? |
|
Sure, cf JuliaRegistries/General#66559 . Made it a minor version bump, technically it is not exposed API but should make dependency management easier. |
|
Thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The motivation for this PR is that I don't know how to allow custom ForwardDiff tags with
ForwardDiff.checktag(such as in https://github.com/TuringLang/Turing.jl/blob/c0c8bc2af0fa32621ded96afcd3ba1cfabd686aa/src/Turing.jl#L45) with the current_logdensity_closuresetup as I don't know how to get the supertype of the result of_logdensity_closure(::MyParameterizedStruct)without evaluating_logdensity_closure. WithBase.Fix1, however, I can easily defineForwardDiff.checktag(::Type{MyTag{V}}, ::Base.Fix1{typeof(logdensity),<:MyParameterizedStruct}, ::AbstractVector{V})and thereby limit the custom tags to the intended use case.An additional "fix" in this PR: Currently the Zygote backend uses
_logdensity_closureas well which is only defined in the optionally loaded ForwardDiff backend. That works because Zygote depends on and loads ForwardDiff but probably it is safer to not having to rely on such internals.