Skip to content

[Minuit2] Fallback to full Hessian in AnalyticalGradientCalculator if no G2 #20957

Merged
guitargeek merged 3 commits into
root-project:masterfrom
guitargeek:issue-20913
Jan 20, 2026
Merged

[Minuit2] Fallback to full Hessian in AnalyticalGradientCalculator if no G2 #20957
guitargeek merged 3 commits into
root-project:masterfrom
guitargeek:issue-20913

Conversation

@guitargeek
Copy link
Copy Markdown
Contributor

If the user function doesn't claim to implement the diagonal of the
Hessian in an efficient way by overriding HasG2() to return true,
the AnalyticalGradientCalculator should not call G2(), but instead
fall back to extract the diagonal from the full Hessian.

Closes #20913.

A test is implemented in a separate commit, and there is also a third commit with some code simplification.

If the user function doesn't claim to implement the diagonal of the
Hessian in an efficient way by overriding `HasG2()` to return `true`,
the AnalyticalGradientCalculator should not call `G2()`, but instead
fall back to extract the diagonal from the full Hessian.

Closes root-project#20913.
Different code branches for limits and no limits are redundant in the
AnalyticalGradientCalculator, because `DInt2Ext` already does that
internally.

It's better to simplify the code to reduce the margin for error.
@guitargeek
Copy link
Copy Markdown
Contributor Author

@AdrianDBS, to which release branches do you need this backported?

And does this also fix #20665?

@guitargeek guitargeek changed the title [Minuit2] Fallback to full Hessian in AnalyticalGradientCalc. if no G2 [Minuit2] Fallback to full Hessian in AnalyticalGradientCalculator if no G2 Jan 19, 2026
@github-actions
Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 12h 0m 46s ⏱️
 3 758 tests  3 757 ✅ 0 💤 1 ❌
75 646 runs  75 645 ✅ 0 💤 1 ❌

For more details on these failures, see this check.

Results for commit ec3de2a.

@AdrianDBS
Copy link
Copy Markdown

@AdrianDBS, to which release branches do you need this backported?

Would be great if this could be backported to 6.36.

And does this also fix #20665?

I will test this today. Yet, while it will most certainly fix the inconsistency described in the first comment in the ticket, I don't think it will solve the main issue described there.

@dpiparo
Copy link
Copy Markdown
Member

dpiparo commented Jan 20, 2026

the mac26 failure seems spurious.

@guitargeek
Copy link
Copy Markdown
Contributor Author

the mac26 failure seems spurious.

Indeed it's a sporadic failure that we have suppressed in the past (with #19043), but in commit 50ae29d I have re-enabled such sporadically failing tests again so that we see if they are still there and if yes address them. The problem with disabling these tests is that we won't know if we regress with the cppyy upgrade.

@guitargeek
Copy link
Copy Markdown
Contributor Author

@AdrianDBS, thanks for trying it out, I'm looking forward to your report! The backport to 6.36 won't be a problem.

Copy link
Copy Markdown
Member

@hageboeck hageboeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@guitargeek guitargeek merged commit f842f2a into root-project:master Jan 20, 2026
34 of 36 checks passed
@guitargeek guitargeek deleted the issue-20913 branch January 20, 2026 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MIGRAD can call G2 even if HasG2 is set to false

4 participants