Throw runtime error when parameter is set via command line in a range…

[maxgalli]

… outside its value

Summary by CodeRabbit

• Bug Fixes

  • Added post-assignment validation for numeric parameters so assigned values are verified and must fall within defined ranges; out-of-range assignments now raise a clear runtime error with guidance to adjust ranges or use range-setting utilities.
  • Applies to both pattern-based and direct parameter assignments.

• Tests

  • Added test cases verifying failure behavior when parameters are set outside allowed ranges.

[maxgalli]

To be discussed before merging

[coderabbitai]

📝 Walkthrough

Walkthrough

Post-assignment validation was added in src/utils.cc for RooRealVar values in both regex-based and direct-assignment paths; if the assigned value is clipped to the variable's [min,max], a runtime_error is thrown instructing range adjustment.

Changes

Cohort / File(s)	Summary
Parameter Range Validation src/utils.cc	Added post-set checks after assigning values to RooRealVar in both rgx{...} and direct-assignment paths; if the actual value differs from the requested value due to clipping to [min,max], throw runtime_error advising use of rMin/rMax or setParameterRanges.
Tests — out-of-range cases test/CMakeLists.txt	Added two tests (setParameters-out-of-range, setParameters-out-of-range-regex) that run MultiDimFit with out-of-range parameter assignments and expect failure with message matching "outside its range".

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I nibble on code and check each bound,
If a value's clipped, I thump the ground,
A runtime cry to set ranges right,
Hop, adjust rMin/rMax by moonlight,
— From your diligent rabbit. 🐇

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title partially captures the main change (parameter range validation and runtime error), but is incomplete and cuts off mid-sentence.	Complete the title to fully convey the intent, e.g., 'Throw runtime error when parameter is set via command line outside its value range' or 'Validate parameter bounds when set via command line and throw error if out of range'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

No actionable comments were generated in the recent review. 🎉

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/utils.cc`:
- Around line 799-806: The code block around
SetParameterExpression/PhysicsParameterValue/tmpParameter (involving
tmpParameter->setVal(...) and the throw std::runtime_error(Form(...))) has
clang-format violations; run the auto-formatter (git-clang-format HEAD~) to
reformat this region so it meets project style rules, then stage the formatted
changes and push; ensure the lines with the cout, tmpParameter->setVal, and the
throw/Form call are reformatted consistently.

🧹 Nitpick comments (1)

src/utils.cc (1)

768-772: Consider extracting a small helper to reduce duplication.

Both the regex and direct-assignment paths share the same post-setVal validation and error-throwing logic. A small inline helper (or even a lambda) would keep them in sync if the message or logic ever changes.

♻️ Example helper extraction

+namespace {
+    void validateParameterInRange(RooRealVar *param, double requestedVal, const std::string &name) {
+        if (param->getVal() != requestedVal) {
+            throw std::runtime_error(Form("Parameter %s value %g is outside its range [%g, %g]. "
+                      "Use --rMin/--rMax or --setParameterRanges to adjust the range.",
+                      name.c_str(), requestedVal, param->getMin(), param->getMax()));
+        }
+    }
+}  // namespace

Then in both call sites:

                     tmpParameter->setVal(PhysicsParameterValue);
-                    if (tmpParameter->getVal() != PhysicsParameterValue) {
-                        throw std::runtime_error(Form("Parameter %s value %g is outside its range [%g, %g]. "
-                                  "Use --rMin/--rMax or --setParameterRanges to adjust the range.",
-                                  target.c_str(), PhysicsParameterValue, tmpParameter->getMin(), tmpParameter->getMax()));
-                    }
+                    validateParameterInRange(tmpParameter, PhysicsParameterValue, target);

Also applies to: 802-806

[codecov]

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 20.81%. Comparing base (0e2dbb7) to head (7c37d2d).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/utils.cc	92.85%	1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (92.85%) is below the target coverage (98.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1209      +/-   ##
==========================================
+ Coverage   20.71%   20.81%   +0.09%     
==========================================
  Files         195      195              
  Lines       26166    26178      +12     
  Branches     3924     3926       +2     
==========================================
+ Hits         5420     5448      +28     
+ Misses      20746    20730      -16     

Files with missing lines	Coverage Δ
src/utils.cc	41.40% <92.85%> (+2.61%)	⬆️

... and 1 file with indirect coverage changes

Files with missing lines	Coverage Δ
src/utils.cc	41.40% <92.85%> (+2.61%)	⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[anigamova]

Are there any other reason why the tmpParameter->setValue() might now work here? I maybe we could check that the parameter is within ranges instead of the current logic?