Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SiStripHashedDetId constructor / operator and add more SiStripLorentzAngle PCL unit testings #43469

Conversation

mmusich
Copy link
Contributor

@mmusich mmusich commented Dec 1, 2023

PR description:

This PR is a follow up to #43423 and implements several different fixes and improvements.
First it fixes the SiStripHashedDetId class, both in terms of the copy constructor (5b7f10a) and assignment operator (3eda318). It also adds a new size() public method (349ac88).
Then it adds a catch2-based unit test for basic SiStripHashedDetId operations (done in commits f20b63e, 8b2231d and 5f34bdd).
Finally it improves the filling of the per-module histograms in SiStripLorentzAnglePCLMonitor (commits 6fb57ae, 6e38f12) as well as adding a dedicated unit test of both the AlCASkimming and AlCaHarvesting steps (519df2c).

PR validation:

The validation of this PR relies on running the augment battery of unit tests of this package.
Running scam b runtests returns

Creating test log file logs/el8_amd64_gcc12/testing.log
Pass    0s ... CalibFormats/SiStripObjects/TestCalibFormatsSiStripObjects
Pass    5s ... CalibFormats/SiStripObjects/testSiStripHashedDetId
Pass    1s ... CalibFormats/SiStripObjects/test_catch2_SiStripHashedDetId
Pass   43s ... CalibTracker/SiStripLorentzAngle/testPromptCalibProdSiStripLA
>> Test sequence completed for CMSSW CMSSW_14_0_X_2023-11-30-2300

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, but might be backported in case there is prolonged cosmics data-taking in 13_3_X foreseen at some point.

@mmusich
Copy link
Contributor Author

mmusich commented Dec 1, 2023

type bug-fix

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43469/38025

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2023

A new Pull Request was created by @mmusich (Marco Musich) for master.

It involves the following packages:

  • CalibFormats/SiStripObjects (alca)
  • CalibTracker/SiStripLorentzAngle (alca)

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

cms-bot commands are listed here

@mmusich
Copy link
Contributor Author

mmusich commented Dec 1, 2023

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-58b2f7/36249/summary.html
COMMIT: 5f34bdd
CMSSW: CMSSW_14_0_X_2023-11-30-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43469/36249/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

@perrotta
Copy link
Contributor

perrotta commented Dec 1, 2023

+1

  • Besides the already known spurious differences in outputs (see github issues #43456, #43293, #39803) everything works, including the new unit test added here.

@cmsbuild
Copy link
Contributor

cmsbuild commented Dec 1, 2023

This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @sextonkennedy, @antoniovilela, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@rappoccio
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit be2dc8e into cms-sw:master Dec 1, 2023
11 checks passed
@mmusich mmusich deleted the my-cmssw/mm_dev_addSiStripLorentzAnglePCL_unit_tests_14_0_X branch December 1, 2023 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants