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

Add hgcalv16 modifier and corresponding era #36753

Merged
merged 2 commits into from Jan 31, 2022

Conversation

srimanob
Copy link
Contributor

@srimanob srimanob commented Jan 19, 2022

PR description:

This PR is to add a new modifier for HGCAL V16, and the corresponding era. Currently, HGCal V16 (C17) is used in D86 and D88. It needs new RecHits calibration, introduced in #36728. We need to split the modifier/era of D86 and D88 from C11, in order to keep D77 (default now) working as before.

PR validation:

This PR itself should do nothing. To see the effect of new RecHit calibration, it should be triggerred the test with #36728

if this PR is a backport please specify the original PR and why you need to backport that PR:

No need of backporting.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36753/27866

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @srimanob (Phat Srimanobhas) for master.

It involves the following packages:

  • Configuration/Eras (operations)
  • Configuration/PyReleaseValidation (pdmv, upgrade)
  • Configuration/StandardSequences (operations)

@perrotta, @jordan-martins, @bbilin, @wajidalikhan, @cmsbuild, @AdrianoDee, @srimanob, @kskovpen, @qliphy, @fabiocos, @davidlange6 can you please review it and eventually sign? Thanks.
@fabiocos, @makortel, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @Martin-Grunewald, @missirol, @kpedro88, @lecriste, @mtosi, @ebrondol, @mmusich, @dgulhan, @slomeo this is something you requested to watch as well.
@perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@@ -1144,7 +1144,7 @@
'from Geometry.EcalMapping.EcalMapping_cfi import *',
'from Geometry.EcalMapping.EcalMappingRecord_cfi import *',
],
"era" : "phase2_ecal, phase2_hcal, phase2_hgcal, hcalHardcodeConditions, phase2_hgcalV10, phase2_hgcalV11, phase2_hgcalV12, phase2_hfnose",
"era" : "phase2_ecal, phase2_hcal, phase2_hgcal, hcalHardcodeConditions, phase2_hgcalV10, phase2_hgcalV11, phase2_hgcalV16, phase2_hfnose",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not this line that we need to keep V10,V11 and V12 or not. Will look into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kpedro88 Do you have an idea what is the result of this era line? I think it does not change anything at the moment. Thanks in advance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@srimanob indeed, right now it is just printed as a recommendation when a user runs the geometry script. The actual modifiers for a workflow have to be implemented manually.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-36753/27867

@cmsbuild
Copy link
Contributor

@srimanob
Copy link
Contributor Author

@cmsbuild please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-5c9688/21818/summary.html
COMMIT: 1b09a9d
CMSSW: CMSSW_12_3_X_2022-01-19-1100/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/36753/21818/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 4.764.76_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT/step2_ZMuSkim2012D+ZMuSkim2012D+HLTDSKIM2+RECODR1reHLT2+HARVESTDR1reHLT.log

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 43
  • DQMHistoTests: Total histograms compared: 3464860
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3464832
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 42 files compared)
  • Checked 181 log files, 42 edm output root files, 43 DQM output files
  • TriggerResults: no differences found

@srimanob
Copy link
Contributor Author

As described, no change is expected from this PR itself. The test with HGCal V16 rechit calibration is done in #36728 (comment)

@srimanob
Copy link
Contributor Author

+Upgrade

The failure in PR test doest not relate to this PR. PR test alone shows no change as expected. Testing with #36728 shows expected behaviour, #36728 (comment)

@cvuosalo
Copy link
Contributor

+1

@srimanob
Copy link
Contributor Author

Kindly ping @cms-sw/pdmv-l2

@kskovpen
Copy link
Contributor

+1

@perrotta
Copy link
Contributor

perrotta commented Jan 31, 2022

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs (but tests are reportedly failing). This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

+1

@perrotta
Copy link
Contributor

merge

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.

None yet

6 participants