Skip to content

fix: overloaded_input_type for one-element vector input#954

Merged
gdalle merged 2 commits intoJuliaDiff:mainfrom
ErikQQY:qqy/one_element
Jan 24, 2026
Merged

fix: overloaded_input_type for one-element vector input#954
gdalle merged 2 commits intoJuliaDiff:mainfrom
ErikQQY:qqy/one_element

Conversation

@ErikQQY
Copy link
Copy Markdown
Contributor

@ErikQQY ErikQQY commented Jan 22, 2026

overloaded_input_type doesn't work for jacobian with one-element vector:

julia> backend = AutoForwardDiff()
AutoForwardDiff()

julia> DI.overloaded_input_type(prepare_jacobian(copy, backend, [1.0]))
ERROR: BoundsError: attempt to access 1-element Vector{ForwardDiff.Dual{ForwardDiff.Tag{typeof(copy), Float64}, Float64, 1}} at index [2]

@ErikQQY ErikQQY requested a review from gdalle as a code owner January 22, 2026 10:13
@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.19%. Comparing base (cd515b3) to head (44d8e95).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #954   +/-   ##
=======================================
  Coverage   98.19%   98.19%           
=======================================
  Files         133      133           
  Lines        7982     7983    +1     
=======================================
+ Hits         7838     7839    +1     
  Misses        144      144           
Flag Coverage Δ
DI 98.96% <100.00%> (+<0.01%) ⬆️
DIT 96.19% <ø> (ø)

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 Jan 23, 2026

Hi @ErikQQY, thanks for the suggested bug fix!
However, it turns out the issue runs deeper than that, and actually goes back to our first PR introducing this mechanism in #672. We neglected several aspects back then:

  • from DerivativeConfig in the two-argument case (in-place), we used to retrieve the type of ydual but we actually want the type of xdual.
  • from JacobianConfig in the one-argument case (out-of-place), we tried to retrieve the type of xduals based on this definition but we ended up pulling xduals[2] because of that definition.
  • some of the tests were wrong too

I tried to fix all of that, would you mind giving it a second, closer look, comparing it with https://github.com/JuliaDiff/ForwardDiff.jl/blob/master/src/config.jl#L159?

Thanks

@ErikQQY
Copy link
Copy Markdown
Contributor Author

ErikQQY commented Jan 24, 2026

I think the current fix looks good now. The previous implementation mess up different cases😅

@gdalle gdalle merged commit 5996df5 into JuliaDiff:main Jan 24, 2026
68 of 69 checks passed
@ErikQQY ErikQQY deleted the qqy/one_element branch January 24, 2026 20:41
@ErikQQY
Copy link
Copy Markdown
Contributor Author

ErikQQY commented Jan 24, 2026

Can we get a new release with this patch? I need this fix for some new functionalities.

@gdalle
Copy link
Copy Markdown
Member

gdalle commented Jan 26, 2026

I tried to register a new version but Registrator didn't notice

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.

2 participants