Skip to content

Comments

[RF] Remove functions to customize epsilons for range checks#21344

Open
guitargeek wants to merge 1 commit intoroot-project:masterfrom
guitargeek:setValFast
Open

[RF] Remove functions to customize epsilons for range checks#21344
guitargeek wants to merge 1 commit intoroot-project:masterfrom
guitargeek:setValFast

Conversation

@guitargeek
Copy link
Contributor

@guitargeek guitargeek commented Feb 20, 2026

The RooNumber::setRangeEpsRel() and RooNumber::setRangeEpsAbs() have been introduced 2 years ago in 4863727 to customize range check behavior to be like before ROOT 6.28, but this has not been proven necessary. Instead, looking up the static variables with the epsilon values incurred significant overhead in RooAbsRealLValue::inRange(), which is visible in many-parameter fits.

@github-actions
Copy link

github-actions bot commented Feb 20, 2026

Test Results

    22 files      22 suites   3d 3h 49m 46s ⏱️
 3 798 tests  3 792 ✅ 0 💤 6 ❌
76 400 runs  76 394 ✅ 0 💤 6 ❌

For more details on these failures, see this check.

Results for commit 6eb148f.

♻️ This comment has been updated with latest results.

@dpiparo dpiparo self-requested a review February 22, 2026 16:37
The `RooNumber::setRangeEpsRel()` and `RooNumber::setRangeEpsAbs()` have
been introduced 2 years ago in 4863727 to customize range check
behavior to be like before ROOT 6.28, but this has not been proven
necessary. Instead, looking up the static variables with the epsilon
values incurred significant overhead in `RooAbsRealLValue::inRange()`,
which is visible in many-parameter fits.
@guitargeek guitargeek requested a review from bellenot as a code owner February 22, 2026 19:30
@guitargeek guitargeek changed the title [RF] Use RooRealVar::setValFast() in minimizer interface [RF] Remove functions to customize epsilons for range checks Feb 22, 2026
@guitargeek
Copy link
Contributor Author

Thinking more about it, I changed strategy here. Skipping the range check would be fine for Minuit, but given that we also want to support other minimizers that might not support parameter bounds, it's probably safer to stick with RooRealVar::setVal() to do the range check.

I have instead figured out where the overhead comes from and suggest to eliminate it in this PR. It requires removing some public functions without a deprecation period, but given that these functions were only introduced in 2023 as an escape hatch to get old behavior, it should be fine to remove them as long as announced in the release notes.

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