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

Phase2-gex134 Modify C17 to see the new numbering of HCAL #37951

Merged
merged 1 commit into from Sep 14, 2022

Conversation

bsunanda
Copy link
Contributor

PR description:

Modify C17 to see the new numbering of HCAL

PR validation:

Use the runTheMatrix test workflows

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

othing special

@cmsbuild cmsbuild added this to the CMSSW_12_4_X milestone May 15, 2022
@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-37951/30018

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @bsunanda (Sunanda Banerjee) for master.

It involves the following packages:

  • Configuration/Geometry (geometry, upgrade)
  • Geometry/CMSCommonData (geometry, upgrade)

@civanch, @Dr15Jones, @makortel, @cvuosalo, @ianna, @mdhildreth, @AdrianoDee, @srimanob can you please review it and eventually sign? Thanks.
@vargasa, @kpedro88, @fabiocos, @Martin-Grunewald, @missirol, @trtomei, @beaucero, @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

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-46e449/24726/summary.html
COMMIT: 571f8b0
CMSSW: CMSSW_12_4_X_2022-05-15-0000/slc7_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37951/24726/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

@slava77 comparisons for the following workflows were not done due to missing matrix map:

  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-46e449/11634.301_TTbar_14TeV+2021_Run3FS+TTbar_14TeV_TuneCP5_GenSim+HARVESTNano

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3741432
  • DQMHistoTests: Total failures: 92
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3741318
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@cvuosalo
Copy link
Contributor

+1

@srimanob
Copy link
Contributor

srimanob commented May 16, 2022

Do we expect to see any changes in Phase-2 D88 in both DDD and DD4hep from this PR? Just to make sure to understand the effect of this PR. Thanks.

@bsunanda
Copy link
Contributor Author

This make changes to some rare process. I am not sure if the difference can be seen in tests with small samples

@srimanob
Copy link
Contributor

+Upgrade

This fix follows the validation report of HCAL [*] and it should be the target for the next Phase-2 DD4hep validation. However, it seems unclear to me if the change is expected also for DDD as file Geometry/CMSCommonData/python/cmsExtendedGeometry2026D86XML_cfi.py
also changes. The description of validation campaign should be clear.

[*]
https://cms-pdmv.cern.ch/valdb/?srch=dd4hep&selected=CMSSW_12_4_0_pre3_D88_DD4HEP&Reconstruction=true&RFull=true

@cmsbuild
Copy link
Contributor

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. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@perrotta
Copy link
Contributor

+Upgrade

This fix follows the validation report of HCAL [*] and it should be the target for the next Phase-2 DD4hep validation. However, it seems unclear to me if the change is expected also for DDD as file Geometry/CMSCommonData/python/cmsExtendedGeometry2026D86XML_cfi.py also changes. The description of validation campaign should be clear.

[*] https://cms-pdmv.cern.ch/valdb/?srch=dd4hep&selected=CMSSW_12_4_0_pre3_D88_DD4HEP&Reconstruction=true&RFull=true

@srimanob will you take care of notifying it to @cms-sw/pdmv-l2 , so that it can be correctly taken into account in that validation campaign?

@bsunanda
Copy link
Contributor Author

bsunanda commented Sep 4, 2022

@srimanob Please uphold this PR. It makes changes in the right direction

@srimanob
Copy link
Contributor

srimanob commented Sep 4, 2022

unhold

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 2022

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. @perrotta, @dpiparo, @rappoccio (and backports should be raised in the release meeting by the corresponding L2)

@srimanob
Copy link
Contributor

srimanob commented Sep 4, 2022

@cmsbuild please test workflow 39434.911

@srimanob
Copy link
Contributor

srimanob commented Sep 4, 2022

Hi @bsunanda

It will be great if you could help to understand more the situation.

  • Is this PR a bug-fix or to improve?
  • As mention here, this PR does not fix the issue of bad hcal id in DIGI and RECO step. What is exactly the change that this PR makes.
  • Should it go to C18, C19, C20 also? Also backport for 12_5 phase-2 production?

Thanks.

@bsunanda
Copy link
Contributor Author

bsunanda commented Sep 4, 2022

Hi, this is a bug fix and should be included in all Phase 2 scenarios of CMS. Once this is integrated I can update all calorimeter descriptions with this

@srimanob
Copy link
Contributor

srimanob commented Sep 4, 2022

type bug-fix

@cmsbuild
Copy link
Contributor

cmsbuild commented Sep 4, 2022

-1

Failed Tests: RelVals-INPUT
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-46e449/27312/summary.html
COMMIT: 571f8b0
CMSSW: CMSSW_12_6_X_2022-09-04-0000/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/37951/27312/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-INPUT

  • 138.1138.1_PromptCosmics+RunCosmics2021+RECOCOSDPROMPTRUN3+ALCACOSDPROMPTRUN3+HARVESTDCPROMPTRUN3/step2_PromptCosmics+RunCosmics2021+RECOCOSDPROMPTRUN3+ALCACOSDPROMPTRUN3+HARVESTDCPROMPTRUN3.log
  • 138.2138.2_ExpressCosmics+RunCosmics2021+RECOCOSDEXPRUN3+ALCACOSDEXPRUN3+HARVESTDCEXPRUN3/step2_ExpressCosmics+RunCosmics2021+RECOCOSDEXPRUN3+ALCACOSDEXPRUN3+HARVESTDCEXPRUN3.log

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 52
  • DQMHistoTests: Total histograms compared: 3712527
  • DQMHistoTests: Total failures: 57
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3712448
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 51 files compared)
  • Checked 216 log files, 49 edm output root files, 52 DQM output files
  • TriggerResults: no differences found

@srimanob
Copy link
Contributor

srimanob commented Sep 5, 2022

Failure in INPUT does not relate to this PR.

Would it be better if @bsunanda confirm the issue of bad HCAL ID which still show up in PR test (as mentioned before),
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-46e449/27312/runTheMatrix-results/39434.911_TTbar_14TeV+2026D88_DD4hep+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14+DigiTrigger+RecoGlobal+HARVESTGlobal/

@bsunanda
Copy link
Contributor Author

@cmsbuild Please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-46e449/27517/summary.html
COMMIT: 571f8b0
CMSSW: CMSSW_12_6_X_2022-09-13-2300/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/37951/27517/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3618326
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3618302
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit ec65e51 into cms-sw:master Sep 14, 2022
@srimanob
Copy link
Contributor

As this PR merged after 12_6_0_pre2, I would like to remind @cms-sw/pdmv-l2 to make a clear validation statement on the change of D88.

REF, see comment
#37951 (comment)

@kskovpen
Copy link
Contributor

Thanks @srimanob , we will take a note.

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

8 participants