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

Update RPC geometry fix in 2022 and 2023 MC GTs #42139

Merged
merged 2 commits into from
Jul 11, 2023

Conversation

saumyaphor4252
Copy link
Contributor

@saumyaphor4252 saumyaphor4252 commented Jun 29, 2023

PR description:

This PR updates the 2022 and 2023 MC GTs with the RPC geometry fix for the issue in the RPC chambers. More details regarding the tags updated and the original CMS Talk request for the update can be found in [1].

[1] https://cms-talk.web.cern.ch/t/fix-for-rpc-geometry-for-run3-data-and-mc/25678

GT Differences with the last ones are here:

PR validation:

GTs tested locally with runTheMatrix.py -l 12034.0,11634.0,7.23,7.24,159.0,159.1,12434.0,160.1,12834.0 -j 8 --ibeos

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 131X and 130X backports coming up right after

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42139/36119

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @saumyaphor4252 (Saumya Phor) for master.

It involves the following packages:

  • Configuration/AlCa (alca)

@cmsbuild, @tvami, @saumyaphor4252, @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, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@saumyaphor4252
Copy link
Contributor Author

@cmsbuild, please test

@saumyaphor4252
Copy link
Contributor Author

@cmsbuild, please abort

@saumyaphor4252
Copy link
Contributor Author

test parameters:

  • workflows = 12034.0,11634.0,7.23,7.24,159.0,159.1,12434.0,160.1,12834.0

@saumyaphor4252
Copy link
Contributor Author

@cmsbuild, please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6376da/33452/summary.html
COMMIT: 6c74b20
CMSSW: CMSSW_13_2_X_2023-06-29-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42139/33452/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 103 lines to the logs
  • Reco comparison results: 2904 differences found in the comparisons
  • DQMHistoTests: Total files compared: 55
  • DQMHistoTests: Total histograms compared: 3820403
  • DQMHistoTests: Total failures: 5737
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3814644
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 54 files compared)
  • Checked 239 log files, 187 edm output root files, 55 DQM output files
  • TriggerResults: found differences in 1 / 53 workflows

@francescobrivio
Copy link
Contributor

There are a lot of differences, mainly in RPC and Muons directories, which is expected given that we are udpating the RPC geometry, FYI @mileva @jhgoh

But in workflow 12834.0 I see many changes in all directories, especially Tracking, FYI @cms-sw/tracking-pog-l2

  • Maybe because this is a 2024 workflow and this geometry was intended for 2023 only? @bsunanda could you chime in?

@mmusich
Copy link
Contributor

mmusich commented Jul 2, 2023

see many changes in all directories, especially Tracking, FYI

Rather unsurprising, given there are are differences at the level of SiStrip digis link. The question is, if that is expected... I gather you don't (@cms-sw/trk-dpg-l2).

@saumyaphor4252 saumyaphor4252 changed the title Update RPC geometry fix in 2022, 2023 and 2024 MC GTs Update RPC geometry fix in 2022 and 2023 MC GTs Jul 10, 2023
@tvami
Copy link
Contributor

tvami commented Jul 10, 2023

@cmsbuild , please abort

  • actually I want to add more 2022 wfs too

@tvami
Copy link
Contributor

tvami commented Jul 10, 2023

test parameters:

  • workflows = 160, 160.02,159,159.03

@tvami
Copy link
Contributor

tvami commented Jul 10, 2023

@cmsbuild , please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-6376da/33620/summary.html
COMMIT: e5d2c09
CMSSW: CMSSW_13_2_X_2023-07-10-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/42139/33620/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 2 lines to the logs
  • Reco comparison results: 238 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3489946
  • DQMHistoTests: Total failures: 1045
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3488879
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 228 log files, 177 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

@tvami
Copy link
Contributor

tvami commented Jul 10, 2023

I'm not sure what's going on, wf 159 uses the same phase1_2022_realistic_hi GT as the wf 312, one of them shows differences in tracking and the other does not:
Screen Shot 2023-07-10 at 12 11 29

--> i.e. this is not coming from the GT, but given there was 1 week diff from now and last time this was tested, it shouldnt even come from any other PR

@tvami
Copy link
Contributor

tvami commented Jul 10, 2023

chatted a bit with @mandrenguyen he pointed out that one diff between these two is that in one case the GENSIM is reused, while in the other it's regenerated. In case it's reused, I could imagine incompatibilities.

  • Should we merge this and let the HS relvals figure out if there is anything wrong at all?

@mmusich
Copy link
Contributor

mmusich commented Jul 10, 2023

In case it's reused, I could imagine incompatibilities.

Should not this hold true for all wfs using recycled GS? Isn't there any other in the matrix? Also what about PU library used for mixing (generated with the old geometry)?

@bsunanda
Copy link
Contributor

bsunanda commented Jul 11, 2023 via email

@tvami
Copy link
Contributor

tvami commented Jul 11, 2023

+alca

  • I propose we move this to a relVal campaign with proper stats and regenerated GENSIM

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

@mmusich
Copy link
Contributor

mmusich commented Jul 11, 2023

I propose we move this to a relVal campaign with proper stats and regenerated GENSIM

shouldn't it be done upfront? we went through some pains to get the gen beamspot correct in the input samples when it was updated.

@rappoccio
Copy link
Contributor

+1

as discussed at ORP meeting, will merge this and create relvals for higher-stats checks.

@cmsbuild cmsbuild merged commit 0744c8b into cms-sw:master Jul 11, 2023
@mmusich
Copy link
Contributor

mmusich commented Jul 11, 2023

as discussed at ORP meeting, will merge this and create relvals for higher-stats checks.

So we are happy with broken IBs?

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.

7 participants