-
Notifications
You must be signed in to change notification settings - Fork 15
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
Remove automatic extern
in isapprox
?
#7
Comments
we also call |
This was referenced Jan 28, 2020
#20 changes this to only have methods on |
I am inclined towards this and am infact doing it as part of solving #56 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This may be the same issue as "do we want
extern
at all?" (JuliaDiff/ChainRulesCore.jl#56)But I think if we have it, we may need it to be "smarter" if we're going to have
isapprox(a::Differential, b) = isapprox(extern(a), b)
as we currently do.Because we will need to
extern(a)
(or maybe justunthunk
?) the differential type before callingisapprox
anyway ifa
it is not a scalar (e.g. theZero
Matix case in SVD tests: https://github.com/JuliaDiff/ChainRules.jl/blob/8c4985a9a941df165740badfbee83cdf93661742/test/rulesets/LinearAlgebra/factorization.jl#L20-L22)The text was updated successfully, but these errors were encountered: