[17.0.X] improve `SiStripHitEffFromCalibTree` and `SiStripHitResolFromCalibTree` by mmusich · Pull Request #45083 · cms-sw/cmssw

mmusich · 2024-05-28T21:04:40Z

PR description:

Somewhat inspired by cms-sw/cms-bot#2228, trying to debug crashes reported at cms-sw/cms-bot#2228 (comment), improve the modules SiStripHitEffFromCalibTree and SiStripHitResolFromCalibTree:

close input calibTree files after reading them;
delete all new-ly allocated memory;
use named constants for k_END_OF_LAYERS and k_END_OF_LAYS_AND_RINGS in lieu of magic numbers

PR validation:

scram b runtests_testSiStripHitEfficiency and scram b runtests_testSiStripHitResolution run fine after the update.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Not a backport, might be backported to 14.0.X

cmsbuild · 2024-05-28T21:05:05Z

cms-bot internal usage

cmsbuild · 2024-05-28T21:10:33Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45083/40403

This PR adds an extra 40KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CalibTracker/SiStripHitEfficiency/test/testHitEffWorker.py modified in PR(s): [Dummy] Try creating big-enough cache to cause splitting #44368

cmsbuild · 2024-05-28T21:10:53Z

A new Pull Request was created by @mmusich for master.

It involves the following packages:

CalibTracker/SiStripHitEfficiency (alca)
CalibTracker/SiStripHitResolution (alca)

@cmsbuild, @saumyaphor4252, @perrotta, @consuegs can you please review it and eventually sign? Thanks.
@rsreds, @echabert, @gbenelli, @tocheng, @yuanchao, @mmusich, @robervalwalsh this is something you requested to watch as well.
@antoniovilela, @sextonkennedy, @rappoccio you are the release manager for this.

cms-bot commands are listed here

mmusich · 2024-05-28T21:21:15Z

@cmsbuild, please test

cmsbuild · 2024-05-29T00:19:32Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8e7715/39593/summary.html
COMMIT: 75a6c3d
CMSSW: CMSSW_14_1_X_2024-05-28-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/45083/39593/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

You potentially added 1 lines to the logs
Reco comparison results: 4 differences found in the comparisons
DQMHistoTests: Total files compared: 48
DQMHistoTests: Total histograms compared: 3338862
DQMHistoTests: Total failures: 9
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 3338833
DQMHistoTests: Total skipped: 20
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
Checked 202 log files, 165 edm output root files, 48 DQM output files
TriggerResults: no differences found

perrotta · 2024-05-29T06:22:28Z

+    bool debug{false};
+
+    if (!tFile.IsZombie() && tFile.IsWritable() && debug) {
+      // Delete all objects in the file recursively


Why the objects in file are deleted and the files are written and closed only when the debug option is true?

it was an (unsuccessful) attempt to debug #45084. AFAIU normally the destructor of TFileService should do that automatically, but somewhere there is a double deletion.
This piece could be removed.

cmsbuild · 2024-05-29T15:19:29Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45083/40419

This PR adds an extra 40KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File CalibTracker/SiStripHitEfficiency/test/testHitEffWorker.py modified in PR(s): [Dummy] Try creating big-enough cache to cause splitting #44368

Code check has found code style and quality issues which could be resolved by applying following patch(s)

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45083/40419/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45083/40419/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

cmsbuild · 2024-11-22T13:11:35Z

Milestone for this pull request has been moved to CMSSW_15_0_X. Please open a backport if it should also go in to CMSSW_14_2_X.

cmsbuild · 2025-02-07T08:32:15Z

Milestone for this pull request has been moved to CMSSW_15_1_X. Please open a backport if it should also go in to CMSSW_15_0_X.

cmsbuild · 2025-09-10T06:54:56Z

Milestone for this pull request has been moved to CMSSW_16_0_X. Please open a backport if it should also go in to CMSSW_15_1_X.

cmsbuild · 2025-12-18T12:58:33Z

Milestone for this pull request has been moved to CMSSW_16_1_X. Please open a backport if it should also go in to CMSSW_16_0_X.

perrotta · 2026-02-25T09:58:34Z

-1

This PR is stuck since quite some while: let reject it, to keep it out of our radars
As soon as @mmusich or anyone else will pick it up again, the alca signature will be reset, and we will get back to it

cmsbuild · 2026-04-04T12:39:40Z

Milestone for this pull request has been moved to CMSSW_17_0_X. Please open a backport if it should also go in to CMSSW_16_1_X.

cmsbuild · 2026-06-05T16:34:46Z

Milestone for this pull request has been moved to CMSSW_20_0_X. Please open a backport if it should also go in to CMSSW_17_0_X.

makortel · 2026-06-05T19:08:36Z

Just checking, is this PR intended for Phase 2 (20_1_X) or Run 3 legacy (17_0_X) or both?

smuzaffar · 2026-08-18T12:20:48Z

this needs rebase/resolve merge conflicts

improve SiStripHitEffFromCalibTree and SiStripHitResolFromCalibTree

75a6c3d

cmsbuild added this to the CMSSW_14_1_X milestone May 28, 2024

cmsbuild added alca-pending pending-signatures tests-pending orp-pending code-checks-pending trk labels May 28, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels May 28, 2024

cmsbuild added tests-started and removed tests-pending labels May 28, 2024

cmsbuild added tests-approved and removed tests-started labels May 29, 2024

perrotta reviewed May 29, 2024

View reviewed changes

cmsbuild added tests-pending code-checks-pending and removed tests-approved code-checks-approved labels May 29, 2024

cmsbuild added code-checks-rejected and removed code-checks-pending labels May 29, 2024

mmusich force-pushed the mm_dev_improveCalibraTrackerSiStrip branch from bb8ce72 to 08c6681 Compare May 29, 2024 15:21

cmsbuild added code-checks-pending and removed code-checks-rejected labels May 29, 2024

cmsbuild added this to the CMSSW_14_2_X milestone Sep 3, 2024

cmsbuild mentioned this pull request Sep 24, 2024

fix for cms-sw/cmssw#45084 #46111

Merged

cmsbuild modified the milestones: CMSSW_14_2_X, CMSSW_15_0_X Nov 22, 2024

cmsbuild modified the milestones: CMSSW_15_0_X, CMSSW_15_1_X Feb 7, 2025

mmusich marked this pull request as draft August 1, 2025 08:24

cmsbuild modified the milestones: CMSSW_15_1_X, CMSSW_16_0_X Sep 10, 2025

This was referenced May 28, 2026

[ALCA-DB] Drop Geometry/CommonDetUnit package #51075

Merged

Rpc digi dev v9 #47447

Open

Uh oh!

Conversation

mmusich commented May 28, 2024

PR description:

PR validation:

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

Uh oh!

cmsbuild commented May 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmsbuild commented May 28, 2024

Uh oh!

cmsbuild commented May 28, 2024

Uh oh!

mmusich commented May 28, 2024

Uh oh!

cmsbuild commented May 29, 2024

Comparison Summary

Uh oh!

perrotta May 29, 2024

Choose a reason for hiding this comment

Uh oh!

mmusich May 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmsbuild commented May 29, 2024

Uh oh!

cmsbuild commented Nov 22, 2024

Uh oh!

cmsbuild commented Feb 7, 2025

Uh oh!

cmsbuild commented Sep 10, 2025

Uh oh!

cmsbuild commented Dec 18, 2025

Uh oh!

perrotta commented Feb 25, 2026

Uh oh!

cmsbuild commented Apr 4, 2026

Uh oh!

cmsbuild commented Jun 5, 2026

Uh oh!

makortel commented Jun 5, 2026

Uh oh!

smuzaffar commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cmsbuild commented May 28, 2024 •

edited

Loading

mmusich May 29, 2024 •

edited

Loading