-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[Minuit2] Performance and thread safety improvements plus refactoring #18018
base: master
Are you sure you want to change the base?
Conversation
@@ -83,6 +83,8 @@ HessianGradientCalculator::DeltaGradient(const MinimumParameters &par, const Fun | |||
unsigned int startElementIndex = mpiproc.StartElementIndex(); | |||
unsigned int endElementIndex = mpiproc.EndElementIndex(); | |||
|
|||
MnFcnCaller fcnCaller{fFcn}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be a performance improvement when using strategy==2
, where the HessianGradientCalculator is used.
Test Results 18 files 18 suites 4d 16h 48m 46s ⏱️ Results for commit c3ece06. ♻️ This comment has been updated with latest results. |
This makes the code easier to understand, because it is not obfuscated with the virtual type whether the transformation is done or not. For the user of these classes, the only difference was anyway the constructor signature. Now, both constructors are unified in the MnFcn class, and `MnUserFcn.h` is kept as a backwards compatibility header with an alias `using MnUserFcn = MnFcn`.
This class can simply be replaced by using `std::function`, and actually it is not even used anywhere.
The MnSeedGenerator now does this internally.
Just like the input vector `x` needs to be replicated per thread, the function evaluator needs to be instantiated per thread too since it caches the transformed parameters.
See commit descriptions.