Skip to content

Conversation

@anigamova
Copy link
Collaborator

@anigamova anigamova commented Dec 19, 2025

Addresses https://cms-talk.web.cern.ch/t/warning-when-running-fitdiagnostics/140247

Summary by CodeRabbit

  • Bug Fixes
    • Improved diagnostics and uncertainty calculations to yield more consistent nuisance handling and restore expected backend behavior, reducing spurious warnings and improving stability of fit diagnostics.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 19, 2025

Walkthrough

A single parameter was added to the nuisance NLL construction in FitDiagnostics.cc: the combineCreateNLL call now includes offset = true placed before the warnAboutDifferentBackend flag, changing the created NLL object while preserving the surrounding backend restore logic.

Changes

Cohort / File(s) Summary
NLL Offset Parameter Addition
src/FitDiagnostics.cc
Modified the combineCreateNLL call to include an offset = true parameter for nuisance NLL construction, shifting the parameter order while maintaining backend handling logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single-spot parameter addition; check that offset = true is the intended semantics.
  • Verify parameter ordering and that backend restore behavior remains unchanged.

Poem

🐰 A tiny tweak, a joyful hop,
Offset set true — the silence stops,
NLL shaped with one small tune,
Restored backends hum in tune,
Rabbity cheers under the moon 🌙

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Fix arguments in combineCreateNLL call within FitDiagnostics' directly and specifically describes the main change: fixing function arguments in a specific code location.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fitdiag_warnbug

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 11b7776 and 6ef3f53.

📒 Files selected for processing (1)
  • src/FitDiagnostics.cc (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/FitDiagnostics.cc
⏰ 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). (10)
  • GitHub Check: LCG_108 - ROOT 6.36.02
  • GitHub Check: CMSSW_14_0_0_pre1 - ROOT 6.26.11
  • GitHub Check: CMSSW_11_3_4 - ROOT 6.22.09
  • GitHub Check: dev3/latest - ROOT LCG master
  • GitHub Check: CMSSW_14_1_0_pre4 - ROOT 6.30.07
  • GitHub Check: LCG_102 - ROOT 6.26.04
  • GitHub Check: LCG_106 - ROOT 6.32.02
  • GitHub Check: Compile (py3.10, root6.26.4)
  • GitHub Check: Compile (py3.10, root6.32.2)
  • GitHub Check: Compile (py3.12, root6.34.4)

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

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

@codecov
Copy link

codecov bot commented Dec 19, 2025

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 20.69%. Comparing base (f77cd93) to head (6ef3f53).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/FitDiagnostics.cc 0.00% 1 Missing ⚠️

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

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1195   +/-   ##
=======================================
  Coverage   20.69%   20.69%           
=======================================
  Files         195      195           
  Lines       26156    26156           
  Branches     3923     3923           
=======================================
  Hits         5413     5413           
  Misses      20743    20743           
Files with missing lines Coverage Δ
src/FitDiagnostics.cc 3.48% <0.00%> (ø)
Files with missing lines Coverage Δ
src/FitDiagnostics.cc 3.48% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@anigamova anigamova merged commit fb70137 into main Dec 19, 2025
16 of 17 checks passed
@anigamova anigamova deleted the fitdiag_warnbug branch December 19, 2025 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants