Skip to content

[Math] Move gradient methods to IBaseFunction classes #19070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 18, 2025

Conversation

guitargeek
Copy link
Contributor

Move the gradient-related methods from the
IGradientFunctionMultiDimTempl to the IBaseFunctionMultiDimTempl base class, and the same for the 1D version.

This makes the IGradientFunction classes pure "indicator" classes that indicate whether the gradient is implemented by the user or not. The actual function interface is unified in the IBaseFunctions.

This fixes the problem that the override keywords could not be used consistently in I*Function-derived classes where the base class was templated to be either IBaseFunction or IGradientFunction.

Alternative to:

@ferdymercury
Copy link
Collaborator

Thanks for tackling this!

Besides the failing test, roofit/roofit/test/GaussFunction.h needs an override, too

@guitargeek guitargeek force-pushed the ifunction_gradient branch from c6bbb9a to 5bc22e4 Compare June 17, 2025 21:42
@guitargeek guitargeek requested a review from dpiparo as a code owner June 17, 2025 21:42
Copy link

github-actions bot commented Jun 18, 2025

Test Results

    18 files      18 suites   3d 10h 49m 14s ⏱️
 2 856 tests  2 856 ✅ 0 💤 0 ❌
49 966 runs  49 966 ✅ 0 💤 0 ❌

Results for commit c657b8f.

♻️ This comment has been updated with latest results.

Move the gradient-related methods from the
`IGradientFunctionMultiDimTempl` to the `IBaseFunctionMultiDimTempl`
base class, and the same for the 1D version.

This makes the `IGradientFunction` classes pure "indicator" classes that
indicate whether the gradient is implemented by the user or not. The
actual function interface is unified in the IBaseFunctions.

This fixes the problem that the override keywords could not be used
consistently in I*Function-derived classes where the base class was
templated to be either `IBaseFunction` or `IGradientFunction`.
Copy link
Member

@lmoneta lmoneta 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 9062a1f into root-project:master Jun 18, 2025
22 of 24 checks passed
@guitargeek guitargeek deleted the ifunction_gradient branch June 18, 2025 10:27
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.

3 participants