Skip to content

fix jacobian(_, AutoFiniteDiff(fdjtype=Val(:central)))#987

Draft
oscardssmith wants to merge 1 commit intomainfrom
os/fix-finite-central-diff
Draft

fix jacobian(_, AutoFiniteDiff(fdjtype=Val(:central)))#987
oscardssmith wants to merge 1 commit intomainfrom
os/fix-finite-central-diff

Conversation

@oscardssmith
Copy link
Copy Markdown
Member

fixes #983 (comment)
FiniteDiff requires x1==x for central mode autodiff.

FiniteDiff requires `x1==x` for central mode autodiff.
@oscardssmith oscardssmith requested a review from gdalle as a code owner April 2, 2026 18:06
@gdalle
Copy link
Copy Markdown
Member

gdalle commented Apr 2, 2026

If that's true, this is not the correct fix, because preparation can be reused at different points than the original one. The correct fix would be something like copyto!(prep.cache.x1, x) at the beginning of execution. But this seems like something FiniteDiff should do itself, otherwise the cache is not safe to reuse in FiniteDiff either

@gdalle gdalle marked this pull request as draft April 2, 2026 18:13
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 12.35%. Comparing base (a5ecbe0) to head (ec78668).

Files with missing lines Patch % Lines
...xt/DifferentiationInterfaceFiniteDiffExt/onearg.jl 0.00% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (a5ecbe0) and HEAD (ec78668). Click for more details.

HEAD has 59 uploads less than BASE
Flag BASE (a5ecbe0) HEAD (ec78668)
DIT 12 1
DI 51 3
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #987       +/-   ##
===========================================
- Coverage   98.21%   12.35%   -85.86%     
===========================================
  Files         135      124       -11     
  Lines        8000     7763      -237     
===========================================
- Hits         7857      959     -6898     
- Misses        143     6804     +6661     
Flag Coverage Δ
DI 6.54% <0.00%> (-92.43%) ⬇️
DIT 27.12% <ø> (-69.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gdalle
Copy link
Copy Markdown
Member

gdalle commented Apr 2, 2026

Also, we would need to fix all operators and add tests

@ChrisRackauckas
Copy link
Copy Markdown
Member

ChrisRackauckas commented Apr 3, 2026

The actual question is just two things, one, are you passing f_in? https://github.com/JuliaDiff/FiniteDiff.jl/blob/master/src/jacobians.jl#L281-L297 If you're passing f(x) but it's the wrong value, yeah that's a problem. If your problem is "but I don't know how to keep it up to date and ..." then... just don't pass it here and let it evaluate it? It's an interface for skipping an f evaluation, and if that interface doesn't apply to DI, then don't use it.

Secondly, the actual issue from the user looks like just uninitialized memory. FiniteDiff doesn't zero things in its non-allocating functions because it assumes the caller has setup the memory appropriately and doesn't touch things it doesn't differentiate. The solution is just to J .= 0.

@oscardssmith
Copy link
Copy Markdown
Member Author

@ChrisRackauckas the problem isn't the jacobian. It's x1 which when using :central for differentiation is required to be equal to x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FiniteDiff with :central includes junk

3 participants