Run the likelihood scan for RooMultiPdf in the cvmfs workflow #1111
Conversation
WalkthroughAdds CI steps to run a grid-based combine scan for RooMultiPdf, generate a 1D scan PDF from the grid output, and upload the PDF as a GitHub Actions artifact. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions Runner
participant CS as Combine (singles)
participant CG as Combine (RooMultiPdf grid)
participant P as plot1DScan.py
participant U as Artifact Uploader
Dev->>GH: Push / PR triggers cvmfs-ci workflow
GH->>CS: Run existing combine (singles)
GH->>CG: Run combine -n RooMultiPdfGrid --algo grid --setParameterRanges r=-1,2 --points 80 -P r
CG-->>GH: Produce RooMultiPdfGrid.MultiDimFit...root
GH->>P: Run plot1DScan.py on grid output
P-->>GH: scan_RooMultiPdf.pdf
GH->>U: Upload artifact "Scan for RooMultiPdf"
Estimated code review effort🎯 2 (Simple) | ⏱️ ~6 minutes Poem
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cvmfs-ci.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/cvmfs-ci.yml
[error] 171-171: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cvmfs-ci.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/cvmfs-ci.yml
[error] 171-171: trailing spaces
(trailing-spaces)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/cvmfs-ci.yml (1)
171-174: Gate artifact upload to CMSSW_14 and fix trailing space; add fail-fast on missing fileTo avoid running the upload on non-CMSSW_14 matrices and to fix YAMLlint’s trailing space (Line 173). It’s also safer to error out if the file is unexpectedly missing.
Apply this diff:
- - name: Upload scan_RooMultiPdf.pdf + - name: Upload scan_RooMultiPdf.pdf + if: ${{ startsWith(matrix.CMSSW_VERSION, 'CMSSW_14') }} uses: actions/upload-artifact@v4 with: - name: Scan for RooMultiPdf - path: ${{ github.workspace }}/artifacts/scan_RooMultiPdf.pdf + name: Scan for RooMultiPdf + path: ${{ github.workspace }}/artifacts/scan_RooMultiPdf.pdf + if-no-files-found: error
🧹 Nitpick comments (1)
.github/workflows/cvmfs-ci.yml (1)
53-53: Avoid creating artifacts dir in a container that doesn’t mount the workspaceThis mkdir targets the host path, but the Build Combine container doesn’t mount ${{ github.workspace }}; it creates a directory in the container FS that isn’t visible to the host and is discarded after the step. Create the directory in the step where you actually copy the PDF (RooMultiPdf), after mounting the workspace.
Apply this diff to remove the ineffective mkdir:
- mkdir -p ${{ github.workspace }}/artifactsYou already create the directory in the RooMultiPdf step per the proposed fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/cvmfs-ci.yml(2 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/cvmfs-ci.yml
[error] 167-167: trailing spaces
(trailing-spaces)
[error] 169-169: trailing spaces
(trailing-spaces)
[error] 173-173: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Compile (py3.10, root6.32.2)
- GitHub Check: Compile (py3.12, root6.34.4)
- GitHub Check: Compile (py3.10, root6.26.4)
- GitHub Check: Compile (py3.8, root6.22)
- GitHub Check: Test with CMSSW_14_0_0_pre1 and ROOT v6.26/11
- GitHub Check: Test with CMSSW_14_2_0_pre2_ROOT632 and ROOT v6.32/06
- GitHub Check: Test with CMSSW_14_1_0_pre4 and ROOT v6.30
- GitHub Check: Test with CMSSW_11_3_4 and ROOT v6.22/08
| combine -M MultiDimFit -m 125.38 -n RooMultiPdfGrid --freezeParameters MH --cminDefaultMinimizerStrategy 0 --X-rtd FAST_VERTICAL_MORPH --X-rtd MINIMIZER_freezeDisassociatedParams --X-rtd MINIMIZER_multiMin_maskChannels=2 --algo grid ws_RooMultiPdf.root --setParameterRanges r=-1,2 --points 80 -P r | ||
| plot1DScan.py higgsCombineRooMultiPdfGrid.MultiDimFit.mH125.38.root -o scan_RooMultiPdf | ||
| cp scan_RooMultiPdf.pdf ${{ github.workspace }}/artifacts/. | ||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mount the workspace in the RooMultiPdf container; current cp to ${{ github.workspace }} will fail
The container for this step doesn’t mount the runner workspace, so copying to ${{ github.workspace }}/artifacts will fail. Also, add -P r to plot1DScan for explicit POI and remove trailing spaces flagged by YAMLlint.
Apply this diff within the run block to create the artifacts dir on the mounted path, make the plot deterministic wrt POI, and export the PDF:
- combine -M MultiDimFit -m 125.38 -n RooMultiPdfGrid --freezeParameters MH --cminDefaultMinimizerStrategy 0 --X-rtd FAST_VERTICAL_MORPH --X-rtd MINIMIZER_freezeDisassociatedParams --X-rtd MINIMIZER_multiMin_maskChannels=2 --algo grid ws_RooMultiPdf.root --setParameterRanges r=-1,2 --points 80 -P r
- plot1DScan.py higgsCombineRooMultiPdfGrid.MultiDimFit.mH125.38.root -o scan_RooMultiPdf
- cp scan_RooMultiPdf.pdf ${{ github.workspace }}/artifacts/.
-
+ combine -M MultiDimFit -m 125.38 -n RooMultiPdfGrid --freezeParameters MH --cminDefaultMinimizerStrategy 0 --X-rtd FAST_VERTICAL_MORPH --X-rtd MINIMIZER_freezeDisassociatedParams --X-rtd MINIMIZER_multiMin_maskChannels=2 --algo grid ws_RooMultiPdf.root --setParameterRanges r=-1,2 --points 80 -P r
+ plot1DScan.py higgsCombineRooMultiPdfGrid.MultiDimFit.mH125.38.root -o scan_RooMultiPdf -P r
+ mkdir -p /work/artifacts
+ cp scan_RooMultiPdf.pdf /work/artifacts/Additionally, update the container options for this step to mount the runner workspace (outside this hunk):
# In the RooMultiPdf step
options: ${{ env.docker_opt_ro }} -v ${{ github.workspace }}:/workThis resolves the file export and the YAMLlint trailing-space errors on Lines 167 and 169.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 167-167: trailing spaces
(trailing-spaces)
[error] 169-169: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/cvmfs-ci.yml around lines 166-169, the RooMultiPdf
container step currently tries to cp to ${{ github.workspace }} but the runner
workspace isn't mounted and YAMLlint flags trailing spaces; update the run block
to create and use an artifacts directory on the mounted path (e.g.,
/work/artifacts), add the explicit POI flag -P r to plot1DScan, and remove
trailing spaces on the listed lines; also update the container options for that
RooMultiPdf step (outside this hunk) to mount the workspace by adding options:
${{ env.docker_opt_ro }} -v ${{ github.workspace }}:/work so the cp to
/work/artifacts succeeds.
Summary by CodeRabbit
New Features
Chores