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.

Sign up for GitHub

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

Jump to bottom

[120X] Revert GTs to use older geometry tags to be consistent w ongoing MC campaign #35478

Merged
merged 1 commit into from
Sep 30, 2021
Merged

[120X] Revert GTs to use older geometry tags to be consistent w ongoing MC campaign #35478

merged 1 commit into from
Sep 30, 2021

Conversation

tvami
Copy link
Contributor

@tvami tvami commented Sep 29, 2021 •
edited
Loading

PR description:

It was requested [1] that the autoCond in 12_0_X stays as close to the production GTs as possible, and the MC production for PF calibration using 12_0_0 is using GTs  that don't have the geometry fixes from [2] (neither do the GTs in 12_0_1), thus we are "reverting" to the geometry tags used in these sample, namely to  

Record | Label |  Tag 	
GeometryFileRcd | Extended | XMLFILE_Geometry_120YV1_Extended2021_mc	
IdealGeometryRecord | - |  TKRECO_Geometry_112YV2

The new GTs are the following, and I'm comparing with the GTs at the time of the MC campaign 

 * 120X_mcRun3_2021_design_v6
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/120X_mcRun3_2021_design_v4/120X_mcRun3_2021_design_v6 
 * 120X_mcRun3_2021_realistic_v9
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/120X_mcRun3_2021_realistic_v6/120X_mcRun3_2021_realistic_v9
 * 120X_mcRun3_2021cosmics_realistic_deco_v8
 https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/120X_mcRun3_2021cosmics_realistic_deco_v5/120X_mcRun3_2021cosmics_realistic_deco_v8
 * 120X_mcRun3_2021_realistic_HI_v8
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/120X_mcRun3_2021_realistic_HI_v5/120X_mcRun3_2021_realistic_HI_v8
 * 120X_mcRun3_2023_realistic_v8
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/120X_mcRun3_2023_realistic_v5/120X_mcRun3_2023_realistic_v8  * 120X_mcRun3_2024_realistic_v8
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/120X_mcRun3_2024_realistic_v5/120X_mcRun3_2024_realistic_v8

Differences are the newly introduced BeamSpot tags needed for [3], except the design GT where the BeamSpot was not changed, i.e. we dont see any differences.

HN announcement in [4]

[1] https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/4475/1/1/1/1/1.html
[2] https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/4460.html
[3] #35397
[4] https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/4478.html

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

This is meant for 12_0_X only

cc @srimanob @cms-sw/geometry-l2 @rappoccio

@cmsbuild cmsbuild added this to the CMSSW_12_0_X milestone Sep 29, 2021
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @tvami (Tamas Vami) for CMSSW_12_0_X.

It involves the following packages:

  • Configuration/AlCa (alca)

@yuanchao, @malbouis, @cmsbuild, @tvami, @francescobrivio can you please review it and eventually sign? Thanks.
@Martin-Grunewald, @missirol, @mmusich, @fabiocos, @tocheng 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

@tvami
Copy link
Contributor Author

tvami commented Sep 29, 2021

urgent

@tvami
Copy link
Contributor Author

tvami commented Sep 29, 2021

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ca329/19258/summary.html
COMMIT: fafea98
CMSSW: CMSSW_12_0_X_2021-09-29-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/35478/19258/install.sh to create a dev area with all the needed externals and cmssw changes.

The following merge commits were also included on top of IB + this PR after doing git cms-merge-topic:

You can see more details here:
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ca329/19258/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-8ca329/19258/git-merge-result

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 7332 differences found in the comparisons
  • DQMHistoTests: Total files compared: 39
  • DQMHistoTests: Total histograms compared: 2998564
  • DQMHistoTests: Total failures: 15183
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2983359
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 38 files compared)
  • Checked 165 log files, 37 edm output root files, 39 DQM output files
  • TriggerResults: found differences in 2 / 38 workflows

@francescobrivio
Copy link
Contributor

+alca

  • differences in the comparison are expected due to change in geometry tags
  • @tvami please fix the link of the design GT diff in the PR description as it seems broken (the correct one is this one)

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_12_0_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_1_X is complete. 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

@tvami @francescobrivio I understand that the differences wrt the tags currently in the 12_0_X IBs are understood and expected.
As a sanity check, did anybody verify that there are actually no differences when comparing the newly submitted GTs and the ones in use for the Run3 productions with 12_0_X?

@francescobrivio
Copy link
Contributor

@tvami @francescobrivio I understand that the differences wrt the tags currently in the 12_0_X IBs are understood and expected. As a sanity check, did anybody verify that there are actually no differences when comparing the newly submitted GTs and the ones in use for the Run3 productions with 12_0_X?

@perrotta the differences between the new GTs and the ones used in 12_0 MC production are listed in the PR description by @tvami:

The new GTs are the following, and I'm comparing with the GTs at the time of the MC campaign

@perrotta
Copy link
Contributor

perrotta commented Sep 30, 2021 •
edited
Loading

Ok, the differences are just the additions of those two BeamSpotOnline records in all Run3 GTs but the "design" one, as stated.
I originally wondered whether those additions would have resulted in some unwanted change in output... but yes, I agree that those records relate to the "online" beam spot, and therefore it is expected that they are transparent to those MC based RelVals.
Going to sign this.

@mmusich
Copy link
Contributor

mmusich commented Sep 30, 2021

the differences are just the additions of those two BeamSpotOnline records in all Run3 GTs but the "design" one, as stated.

I realize now that the design one should receive that addition too ... subcritical at best though.

I originally wondered whether those additions would have resulted in some unwanted change in output... but yes, I agree that those records relate to the "online" beam spot, and therefore it is expected that they are transparent to those MC based RelVals.

The "online BS" is consumed even in MC due to the AlCaBeamMonitor module which runs in the standard DQM sequences. If those records are not provided it will just fallback to DB (regular BeamSpot record) and emit a warning as agreed in #35373 (one per IOV , i.e. only once in MC).

@perrotta
Copy link
Contributor

I realize now that the design one should receive that addition too ... subcritical at best though.

Should we wait for it, then?

@mmusich
Copy link
Contributor

mmusich commented Sep 30, 2021 •
edited
Loading

Should we wait for it, then?

It'll take me some time to cook the payload. As long as you don't mind having this one warning per job in the relvals there's no hurry. Let me know.

EDIT: for the record here it is:

https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/slc7_amd64_gcc900/CMSSW_12_1_X_2021-09-29-1100/pyRelValMatrixLogs/run/12034.0_TTbar_14TeV+2021Design+TTbar_14TeV_TuneCP5_GenSimINPUT+Digi+Reco+HARVEST/step3_TTbar_14TeV+2021Design+TTbar_14TeV_TuneCP5_GenSimINPUT+Digi+Reco+HARVEST.log

It appears actually multiple times I guess because the job is multi-threaded.

@francescobrivio
Copy link
Contributor

@perrotta I think that having a warning per job in the relvals is acceptable for the time being and given the hurry in building the new 12_0_X release I would merge this PR asap.

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit f98f707 into cms-sw:CMSSW_12_0_X Sep 30, 2021
@tvami
Copy link
Contributor Author

tvami commented Sep 30, 2021

  • @tvami please fix the link of the design GT diff in the PR description as it seems broken (the correct one is this one)

I fixed this, there was an extra * got into the link from the next line in the enumeration

This was referenced Oct 3, 2021
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
alca-approved fully-signed orp-approved tests-approved urgent
Projects
None yet
Milestone
CMSSW_12_0_X
Development

Successfully merging this pull request may close these issues.
5 participants
@tvami @cmsbuild @francescobrivio @perrotta @mmusich