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

[RFC] Add updated but "off" FastSim pixel resolution histograms infrastructure #34812

Closed

Conversation

lcorcodilos
Copy link

PR description:

This PR adds the infrastructure to use the FastSim pixel resolution histograms that were recently submitted to cms-data in this PR [1]. These are based on UL pixel simulation but will not be used in the UL FastSim production which has already begun.

This PR is just meant to preserve the work that was done and to make the new resolution histograms easily accessible in the future with the hard-coded boolean ULresolutionHistograms which is set to False. As result, this PR should produce no change.

[1] - cms-data/FastSimulation-TrackingRecHitProducer#4

Known issues/need for feedback

The new resolution histograms have been prepared for each year but the modifiers used to this point have been "per-phase". So in the case of Phase 1, the modifier needs to be different depending on the era. I currently have it coded as

    from Configuration.Eras.Era_Run2_2017_FastSim_cff import Run2_2017_FastSim
    Run2_2017_FastSim.toModify(fastTrackerRecHits, plugins = pixelPlugins2017UL)

    from Configuration.Eras.Era_Run2_2018_FastSim_cff import Run2_2018_FastSim
    Run2_2018_FastSim.toModify(fastTrackerRecHits, plugins = pixelPlugins2018UL)

but this obviously does not work - it just conveys the intention. We could use feedback on the best way to accomplish this implementation.

PR validation:

The following have been run successfully:

scram build code-checks
scram build code-format
scram b runtests

I also ran runTheMatrix.py -l limited -i all --ibeos but see failures that appear to be unrelated to these changes.

@cmsbuild cmsbuild added this to the CMSSW_12_1_X milestone Aug 6, 2021
@lcorcodilos
Copy link
Author

@tvami
Copy link
Contributor

tvami commented Aug 6, 2021

@cmsbuild , please test with cms-data/FastSimulation-TrackingRecHitProducer#4

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-34812/24557

  • This PR adds an extra 28KB to repository

  • Found files with invalid states:

    • FastSimulation/TrackingRecHitProducer/python/PixelPluginsUL_cfi.py:

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2021

A new Pull Request was created by @lcorcodilos for master.

It involves the following packages:

  • FastSimulation/Configuration (fastsim)
  • FastSimulation/TrackingRecHitProducer (fastsim)

@lveldere, @civanch, @ssekmen, @mdhildreth, @sbein can you please review it and eventually sign? Thanks.
@matt-komm this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy, @perrotta you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented Aug 6, 2021

-1

Failed Tests: RelVals AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-295416/17613/summary.html
COMMIT: 1204004
CMSSW: CMSSW_12_1_X_2021-08-06-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34812/17613/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

----- Begin Fatal Exception 06-Aug-2021 20:09:01 CEST-----------------------
An exception of category 'FatalRootError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TrackingRecHitProducer label='fastTrackerRecHits'
   Additional Info:
      [a] Fatal Root Error: @SUB=TArrayD::At
index 1 out of bounds (size: 0, this: 0x2b0d1cb514d0)

----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 06-Aug-2021 20:09:01 CEST-----------------------
An exception of category 'FatalRootError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TrackingRecHitProducer label='fastTrackerRecHits'
   Additional Info:
      [a] Fatal Root Error: @SUB=TArrayD::At
index 1 out of bounds (size: 0, this: 0x2b1ea6b119d0)

----- End Fatal Exception -------------------------------------------------

AddOn Tests

----- Begin Fatal Exception 06-Aug-2021 20:16:00 CEST-----------------------
An exception of category 'FatalRootError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TrackingRecHitProducer label='fastTrackerRecHits'
   Additional Info:
      [a] Fatal Root Error: @SUB=TArrayD::At
index 1 out of bounds (size: 0, this: 0x2b2f475f6ad0)

----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 06-Aug-2021 20:16:02 CEST-----------------------
An exception of category 'FatalRootError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TrackingRecHitProducer label='fastTrackerRecHits'
   Additional Info:
      [a] Fatal Root Error: @SUB=TArrayD::At
index 1 out of bounds (size: 0, this: 0x2b87543f4ad0)

----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 06-Aug-2021 20:16:00 CEST-----------------------
An exception of category 'FatalRootError' occurred while
   [0] Constructing the EventProcessor
   [1] Constructing module: class=TrackingRecHitProducer label='fastTrackerRecHits'
   Additional Info:
      [a] Fatal Root Error: @SUB=TArrayD::At
index 1 out of bounds (size: 0, this: 0x2b6f9fc715d0)

----- End Fatal Exception -------------------------------------------------

@lcorcodilos
Copy link
Author

I'll copy one of these tests and try to debug what's going on. Based on the changes, my suspicion is that something is going on in PixelResolutionHistograms.cc/h

@tvami
Copy link
Contributor

tvami commented Jul 10, 2022

Hmmm the results in #34812 (comment) dont match my recollection... my recollection was that in the end we started seeing result in the central wf-s, like 5.1 but now everything upto MsgLogger looks fine. Is that now just the trick with the stats? Or the point is that in the PR the feature now is disabled by default?

@tvami
Copy link
Contributor

tvami commented Jul 10, 2022

the hard-coded boolean ULresolutionHistograms which is set to False. As result, this PR should produce no change.

ahh yes, ok, so the point indeed is that in the PR the feature now is disabled by default

@sbein
Copy link
Contributor

sbein commented Jul 11, 2022

Yes, the ULresolutionHistograms booleans basically suppress all impact until someone can come and figure out why the enabled feature causes significant changes to physics observables, e.g., @pmaksim1 and company.

@tvami
Copy link
Contributor

tvami commented Jul 11, 2022

t until someone can come and figure out why the enabled feature causes significant changes to physics observables, e.g., pmaksim1 and company.

Yes, we have somebody starting to work on it, @ammitra

@smuzaffar
Copy link
Contributor

Milestone for this pull request has been moved to CMSSW_14_0_X.Please open a backport if it should also go in to CMSSW_13_3_X.

@tvami
Copy link
Contributor

tvami commented Nov 6, 2023

@cmsbuild , please close

  • Lucas left the field, there are several things to debug, a new PR can be opened when those issues are resolved

@cmsbuild cmsbuild closed this Nov 6, 2023
@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 6, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-295416/35633/summary.html
COMMIT: 081af5b
CMSSW: CMSSW_13_3_X_2023-11-06-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/34812/35633/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-295416/35633/git-recent-commits.json
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-295416/35633/git-merge-result

Comparison Summary

Summary:

  • You potentially removed 98 lines from the logs
  • Reco comparison results: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 50
  • DQMHistoTests: Total histograms compared: 3363010
  • DQMHistoTests: Total failures: 1214
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3361774
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 49 files compared)
  • Checked 214 log files, 167 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

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.

6 participants