Support non-Array AbstractVectors for AutoEnzyme hess and fgh!#1274
Support non-Array AbstractVectors for AutoEnzyme hess and fgh!#1274wsmoses wants to merge 7 commits into
Conversation
| function hess(res, θ, p = p) | ||
| Enzyme.make_zero!(bθ) | ||
| Enzyme.make_zero!.(vdbθ) | ||
| θ_arr = θ isa Array ? θ : Array(θ) |
There was a problem hiding this comment.
This isn't correct, it's coercing into an array which for example would not be the correct behavior on GPUArrays.
There was a problem hiding this comment.
this is the existing behavior here:
vdθ = Tuple((Array(r) for r in eachrow(I(length(x)) * one(eltype(x)))))
already does the conversion [we just need to match this for both primal+shadow]
There was a problem hiding this comment.
well then that's the incorrect part that should get fixed.
There was a problem hiding this comment.
how? you cannot guarantee the primal/shadow are the same type [and index offsets] in a nice way that I can think of offhand otherwise (ofc longer term you can make a helper to adapt the array type, but that's a follow up)
There was a problem hiding this comment.
make_zero, Adapt.jl, ArrayInterface.restructure
There was a problem hiding this comment.
I don't think any of those work here, but I'll let you continue it
| optprob_view.fgh(G_view, H_view, x_view) | ||
| @test G1 == G_view | ||
| @test H1 == H_view | ||
|
|
There was a problem hiding this comment.
Should test other things like GPUArrays, StaticArrays, ComponentArrays, etc.
There was a problem hiding this comment.
I'll let you take over this PR then
|
also for context iirc this was needed for the second order ode tests in scimlsensitivity |
Found this while doing the second order scimlsensitivity testing