Skip to content

Conversation

guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Jun 5, 2025

The function double ROOT::Math::Minimizer::GlobalCC(unsigned int ivar) was changed to std::vector<double> ROOT::Math::Minimizer::GlobalCC(), always returning the full list of global correlations.

This change was needed because global correlations are not unconditionally computed and cached in the minimizer anymore. Only computing them when calling GlobalCC() avoids unneeded overhead when global correlations are not required.

Copy link

github-actions bot commented Jun 6, 2025

Test Results

    21 files      21 suites   3d 14h 15m 20s ⏱️
 3 686 tests  3 686 ✅ 0 💤 0 ❌
75 541 runs  75 541 ✅ 0 💤 0 ❌

Results for commit 6d18080.

♻️ This comment has been updated with latest results.

@guitargeek guitargeek force-pushed the minu2 branch 2 times, most recently from f3cfe36 to a252256 Compare July 10, 2025 22:26
@guitargeek guitargeek marked this pull request as ready for review October 3, 2025 13:27
The plan is to update the minimizer implementations to not compute the
global correlations unconditionally anymore, but only when the user
actually requests them with `GlobalCC()`. For this, it makes more sense
to return all values at once. Otherwise, we would have to implement some
caching, which is not desirable.
This makes the code less error prone, because initialization can't be
forgotten or be inconsistent between constructors.
For many parameters, the global correlation matrix can be very expensive
to compute. We therefore don't want to calculate it unconditionally, but
only when the user actually requests them by calling
`Minimizer::GlobalCC()`.
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!
The changes seems correct, this would require the computation of the GlobalCC, which requires inverting a matrix, only when requested. It makes then sense to change also the interface to return the full vector of global correlations.

@guitargeek guitargeek merged commit 77951c7 into root-project:master Oct 3, 2025
26 of 27 checks passed
@guitargeek guitargeek deleted the minu2 branch October 3, 2025 19:15
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.

2 participants