Safely initialize the inplace Jacobian cache#160
Safely initialize the inplace Jacobian cache#160JamesWrigley wants to merge 1 commit intoJuliaNLSolvers:masterfrom
Conversation
|
Do you have an MWE with the error you are encountering? I have a strong feeling that the bug is located downstream, because using similar here is not incorrect |
| return OnceDifferentiable(fF, dfF, fdfF, x_seed, F, DF) | ||
| else | ||
| F2 = similar(F) | ||
| F2 = copy(F) |
There was a problem hiding this comment.
Is F2 mutated in the jacobian! call below? Would it still work if F2 can't be mutated anymore? similar is guaranteed to create a mutable array whereas copy may return a static array.
There was a problem hiding this comment.
| F2 = copy(F) | |
| F2 = copyto!(similar(F), F) |
There was a problem hiding this comment.
Fixed in facf227. I attempted to add a test but couldn't figure out how to make it fail outside of LsqFit so I added a comment instead.
This fixes support for BigFloat in LsqFit.jl, ForwardDiff.jl requires that the array is initialized.
|
Thanks. This PR needs a test for the correct behavior. |
|
This workaround might not be necessary: JuliaDiff/ForwardDiff.jl#743 |
|
Would definitely be nice if we didn't have to rely on fixes here |
|
If ForwardDiff doesn't merge the PR I think we need some kind of fix ourselves here |
|
Kristoffer is fine with the ForwardDiff PR and I just increased the test coverage, so I'll merge and release once codecov confirms that all changes are covered. |
|
The BigFloat fix is released in ForwardDiff 1.0.2: JuliaRegistries/General#137312 Edit: Since the Julia compat bound was updated to 1.10, it had to be released as version 1.1.0. The new release is now available in the registry (but may take a few minutes until it is available in all package servers). |
|
Thanks, but luckily upstream could fix it |
4 participants
This fixes support for BigFloat in LsqFit.jl. Making a copy instead of similar ensures that the elements aren't
undef, which is what previous versions did.CC @gdalle