-
Notifications
You must be signed in to change notification settings - Fork 17
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
StaticArrays woes #413
Comments
For those cases in which you see improper allocations, can you benchmark again using DI's preparation functionality, as outlined in our docs? That's how you can get fully non allocating behavior e.g. for ForwardDiff |
As far as I can tell, for the most part what (Note I realize when re-reading this that I'm conflating To further elucidate this point, note ◖◗ @btime ForwardDiff.gradient($f1, $x)
10.972 ns (0 allocations: 0 bytes)
4-element SVector{4, Float64} with indices SOneTo(4):
2.0
2.0
2.0
2.0 It seems reasonable to me to expect this to behave the same way as |
That's where you're at least partly wrong. My priority in designing this package was to obtain optimal performance after preparation. So by default, DI.gradient (when called without extras) first prepares the operator (creating an extras object) and then calls the gradient with said preparation. I can also make DI.gradient call ForwardDiff.gradient directly while skipping preparation, which would probably give you the behavior you want |
It just wasn't a priority because I thought most performance-focused users would leverage preparation |
It's looking like #414 entirely solves the forward diff cases. |
Hi @ExpandingMan. I have recently given a lot of thought to StaticArrays and I think we are now able to close this issue. EnzymeThis is a whole can of worms and evolving fast, so we should probably open specific issues when the need arises. See also #558 ForwardDiffThe finishing touch is #571, which should make FiniteDiffI actually don't think StaticArrays are supported by their native gradient function either. In any case, the behavior of both packages seems coherent: using ADTypes
import DifferentiationInterface as DI
using FiniteDiff: FiniteDiff
using StaticArrays
x = SVector(1.0, 2.0)
DI.gradient(sum, AutoFiniteDiff(), x) # errors
FiniteDiff.finite_difference_gradient(sum, x) # errors
DI.jacobian(identity, AutoFiniteDiff(), x) # works
FiniteDiff.finite_difference_jacobian(identity, x) # works
DI.hessian(sum, AutoFiniteDiff(), x) # works
FiniteDiff.finite_difference_hessian(sum, x) # works TestingThanks to #571, DifferentiationInterfaceTest works better with StaticArrays. You can even benchmark code in a few lines to check where allocations occur: using DifferentiationInterface, DifferentiationInterfaceTest
using ForwardDiff: ForwardDiff
using StaticArrays
backends = [AutoForwardDiff()]
scenarios = static_scenarios() # modify this with your own scenarios
data = benchmark_differentiation(backends, scenarios; logging=true) # returns a DataFrame |
Thanks. Indeed I've also been losing track, recently some of the hacks I'd been using to fix things have been broken but sometimes I still see some allocations I haven't tracked down yet. It'll probably take a couple of weeks for everything to sort itself out and me to again understand everything that's going on. I will open specific issues if and as I find them, if I find issues in other repos relevant to this I will ping you. |
There currently seem to be quite a few issues with StaticArrays on many different back-ends. In most cases this is inefficiency due to inappropriate allocations (sometimes quite severe), in other cases there are outright errors. This issue is to document the various problems. Note that very few, if any of these are actually issues with DifferentiationInterface.jl itself, but rather with the back-ends.
In what follows we will use
Enzyme
Improper Allocation in Gradient
This is likely due to insufficient specialization in
Enzyme.gradient
for StaticArrays. I have confirmed that a rawEnzyme.autodiff
is efficient and does not allocate. I'm attempting to address this, among other things in this PR.Invalid Construction in Jacobian
This error occurs when an insufficiently narrow StaticArrays type is used as a constructor. Again, this calls for more specialization within Enzyme. This may be fixed by this PR.
ForwardDiff (solved by #414 !)
Improper Allocation In Gradient
This seems to be due to type instability in
ForwardDiff.GradientConfig
which that package mostly relies on the compiler eliding, but which does not get elided during its use in DifferentiationInterface.jl. In my opinion that flaw runs pretty deep in ForwardDiff.jl as it plays fast and loose with types which can only be inferred at runtime, but there is a patch that I believe would fix the this secific issue here.Improper Allocation in Jacobian
I think this is the same issue as the above.
FiniteDiff
I'm less familiar with the internals of this package, but it claims to be non-allocating and compatible with StaticArrays.
Gradient uses
setindex!
This one might actually be a problem with DifferentiationInterface.jl itself because there are surely methods somewhere in FiniteDiff that don't rely on this.
Inappropriate allocations in jacobian
I'm less than completely confident this is indeed a bug, but it likely is as I don't really see why this would have to allocate.
TODO: Others?
There are definitely lots of similar issues with other backends, but I haven't documented them yet. However, many of those other back-ends give fewer guarantees about performance with StaticArrays, so there are likely only 4 or 5 backends (including those listed above) where performance quips are valid.
The text was updated successfully, but these errors were encountered: