[test] Move RooParametricHist test to CTest#1228
Draft
guitargeek wants to merge 1 commit intocms-analysis:mainfrom
Draft
[test] Move RooParametricHist test to CTest#1228guitargeek wants to merge 1 commit intocms-analysis:mainfrom
guitargeek wants to merge 1 commit intocms-analysis:mainfrom
Conversation
Contributor
📝 WalkthroughWalkthroughRemoved the RooParametricHist CI workflow step from GitHub Actions, enabled the RooParametricHist (parametric_hist) test in CMake, and standardized shape-file references and parameter declarations in the datacard to use relative paths. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
test/CMakeLists.txt (1)
257-257: Nitpick: inconsistent indentation oftext2workspace.pyafterT2W_COMMAND.Every other
ADD_COMBINE_TESTblock in this file indents the command body by 4 spaces relative to the keyword (e.g., lines 151, 173, 186, 195). Heretext2workspace.pysits at the same indent level asT2W_COMMAND.♻️ Proposed fix
T2W_COMMAND - text2workspace.py -P HiggsAnalysis.CombinedLimit.PhysicsModel:multiSignalModel --PO verbose --PO map=.*/*hcc*:r[1,-500,500] --PO map=.*/zcc:z[1,-5,5] ${REPO}/data/ci/datacard_RooParametricHist.txt -o ws_RooParametricHist.root + text2workspace.py -P HiggsAnalysis.CombinedLimit.PhysicsModel:multiSignalModel --PO verbose --PO map=.*/*hcc*:r[1,-500,500] --PO map=.*/zcc:z[1,-5,5] ${REPO}/data/ci/datacard_RooParametricHist.txt -o ws_RooParametricHist.root🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/CMakeLists.txt` at line 257, Fix the indentation for the text2workspace.py line in the ADD_COMBINE_TEST block so it matches other blocks: make the text2workspace.py command line (the long command starting with "text2workspace.py -P HiggsAnalysis.CombinedLimit.PhysicsModel:multiSignalModel ...") indented by 4 spaces relative to the T2W_COMMAND keyword, consistent with the indentation style used in other ADD_COMBINE_TEST blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@test/CMakeLists.txt`:
- Line 257: Fix the indentation for the text2workspace.py line in the
ADD_COMBINE_TEST block so it matches other blocks: make the text2workspace.py
command line (the long command starting with "text2workspace.py -P
HiggsAnalysis.CombinedLimit.PhysicsModel:multiSignalModel ...") indented by 4
spaces relative to the T2W_COMMAND keyword, consistent with the indentation
style used in other ADD_COMBINE_TEST blocks.
guitargeek
force-pushed
the
test_parametrichist
branch
from
44f68cb to
e417959
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/CMakeLists.txt`:
- Around line 254-260: The T2W_COMMAND line inside the ADD_COMBINE_TEST block
for parametric_hist is mis-indented (2 spaces) compared to other blocks; open
the ADD_COMBINE_TEST for parametric_hist and adjust the indentation of the
text2workspace.py line so it is aligned with the other command-body lines (4
spaces under T2W_COMMAND) — ensure the text2workspace.py invocation and its
continuation variables (${REPO}/data/ci/datacard_RooParametricHist.txt,
ws_RooParametricHist.root) remain unchanged.
Comment on lines
+254
to
+260
| ADD_COMBINE_TEST(parametric_hist | ||
| COPY_TO_BUILDDIR ${REPO}/data/ci/shapes_RooParametricHist.root | ||
| T2W_COMMAND | ||
| text2workspace.py -P HiggsAnalysis.CombinedLimit.PhysicsModel:multiSignalModel --PO verbose --PO map=.*/*hcc*:r[1,-500,500] --PO map=.*/zcc:z[1,-5,5] ${REPO}/data/ci/datacard_RooParametricHist.txt -o ws_RooParametricHist.root | ||
| COMBINE_COMMANDS | ||
| "combine -M MultiDimFit ws_RooParametricHist.root --algo singles" | ||
| ) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the referenced data files exist
echo "=== Checking for data files ==="
fd -t f "shapes_RooParametricHist.root" 2>/dev/null || echo "shapes_RooParametricHist.root NOT FOUND"
fd -t f "datacard_RooParametricHist.txt" 2>/dev/null || echo "datacard_RooParametricHist.txt NOT FOUND"
# Also check if data/ci/ directory exists and list its contents
echo -e "\n=== Listing data/ci/ directory ==="
if [ -d "data/ci" ]; then
ls -la data/ci/ | head -20
else
echo "data/ci/ directory does not exist"
fiRepository: cms-analysis/HiggsAnalysis-CombinedLimit
Length of output: 1702
🏁 Script executed:
#!/bin/bash
# Verify indentation consistency in test/CMakeLists.txt
# Extract surrounding test blocks to compare indentation patterns
echo "=== Indentation pattern check ==="
sed -n '240,280p' test/CMakeLists.txt | cat -ARepository: cms-analysis/HiggsAnalysis-CombinedLimit
Length of output: 2323
Fix indentation inconsistency in T2W_COMMAND body.
The text2workspace.py command on line 257 is indented 2 spaces while all other ADD_COMBINE_TEST blocks in this file indent the command body 4 spaces under its keyword. Both referenced data files (data/ci/shapes_RooParametricHist.root and data/ci/datacard_RooParametricHist.txt) exist and are accessible.
Suggested indentation fix
ADD_COMBINE_TEST(parametric_hist
COPY_TO_BUILDDIR ${REPO}/data/ci/shapes_RooParametricHist.root
T2W_COMMAND
- text2workspace.py -P HiggsAnalysis.CombinedLimit.PhysicsModel:multiSignalModel --PO verbose --PO map=.*/*hcc*:r[1,-500,500] --PO map=.*/zcc:z[1,-5,5] ${REPO}/data/ci/datacard_RooParametricHist.txt -o ws_RooParametricHist.root
+ text2workspace.py -P HiggsAnalysis.CombinedLimit.PhysicsModel:multiSignalModel --PO verbose --PO map=.*/*hcc*:r[1,-500,500] --PO map=.*/zcc:z[1,-5,5] ${REPO}/data/ci/datacard_RooParametricHist.txt -o ws_RooParametricHist.root
COMBINE_COMMANDS
"combine -M MultiDimFit ws_RooParametricHist.root --algo singles"
)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/CMakeLists.txt` around lines 254 - 260, The T2W_COMMAND line inside the
ADD_COMBINE_TEST block for parametric_hist is mis-indented (2 spaces) compared
to other blocks; open the ADD_COMBINE_TEST for parametric_hist and adjust the
indentation of the text2workspace.py line so it is aligned with the other
command-body lines (4 spaces under T2W_COMMAND) — ensure the text2workspace.py
invocation and its continuation variables
(${REPO}/data/ci/datacard_RooParametricHist.txt, ws_RooParametricHist.root)
remain unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1228 +/- ##
=======================================
Coverage 20.86% 20.86%
=======================================
Files 195 195
Lines 26217 26217
Branches 3932 3932
=======================================
Hits 5469 5469
Misses 20748 20748 🚀 New features to boost your workflow:
|
Collaborator
Author
|
Marked as draft because the differences between ROOT versions need to be understood. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Let's see if it works now.
Summary by CodeRabbit
Tests
Chores