cmssw icon indicating copy to clipboard operation
cmssw copied to clipboard
verified publisher icon cms-sw

ETL hits accumulation on a pixel-basis in MTD Validation

Open rdelliga opened this issue 1 year ago • 26 comments

PR description: The accumulation process in MTD validation, based on modules (DetId), is updated to be pixel-level during SimHits processing, aligning it with the digitization of ETL[1]; in particular, this may impact processes under high pile-up conditions. To be consistent with PR #43762, which changes the ETL digitization, it is important to rectify this discrepancy. A correction attempt has already been made with the failed pull request #43928. The Digi structure (ETLSample) has been also updated to accomodate for one additional parameter, the time over threshold, therefore some plots to monitor Tot are added in EtlDigiHitsValidation. Moreover, debugging verbosity in ETL digitization and adcSaturation_MIP in mtdDigitizer_cfg.py are updated. In ETLUnalibratedRecHitAlgo, amplitude for ETL is set to be the time_over_threshold, because reco energy will not be reconstructed; for the same reason, in EtlLocalecoValidation, some plots about reco energy are removed. Finally, the use of MC truth for tracks and Primary Vertices is updated, to be consistent with PR #43913.

PR validation: The code was tested on the workflow 24834.0_TTbar_14TeV+2026D98

[1] https://github.com/cms-sw/cmssw/blob/master/SimFastTiming/FastTimingCommon/src/ETLDeviceSim.cc

rdelliga avatar Apr 28 '24 08:04 rdelliga

cms-bot internal usage

cmsbuild avatar Apr 28 '24 08:04 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44859/40113

  • This PR adds an extra 76KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File Validation/MtdValidation/plugins/EtlDigiHitsValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/EtlLocalRecoValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/EtlSimHitsValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/MtdTracksValidation.cc modified in PR(s): #43928

Code check has found code style and quality issues which could be resolved by applying following patch(s)

  • code-format: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44859/40113/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44859/40113/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Apr 28 '24 08:04 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44859/40115

  • This PR adds an extra 76KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File Validation/MtdValidation/plugins/EtlDigiHitsValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/EtlLocalRecoValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/EtlSimHitsValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/MtdTracksValidation.cc modified in PR(s): #43928

cmsbuild avatar Apr 28 '24 19:04 cmsbuild

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

It involves the following packages:

  • RecoLocalFastTime/FTLCommonAlgos (reconstruction, upgrade)
  • SimFastTiming/FastTimingCommon (upgrade, simulation)
  • Validation/MtdValidation (dqm)

@jfernan2, @syuvivida, @tjavaid, @antoniovagnerini, @srimanob, @mandrenguyen, @civanch, @cmsbuild, @rvenditti, @subirsarkar, @nothingface0, @mdhildreth can you please review it and eventually sign? Thanks. @fabiocos, @missirol this is something you requested to watch as well. @sextonkennedy, @rappoccio, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

cmsbuild avatar Apr 28 '24 19:04 cmsbuild

please test with workflow 27634.0

fabiocos avatar Apr 29 '24 07:04 fabiocos

type mtd

fabiocos avatar Apr 29 '24 07:04 fabiocos

@cmsbuild please test workflow 27634.0

srimanob avatar Apr 29 '24 08:04 srimanob

-1

Failed Tests: RelVals AddOn Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f3bdfb/39144/summary.html COMMIT: 3bb166835691ae3db068fe2a342d12cd8981866f CMSSW: CMSSW_14_1_X_2024-04-28-2300/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44859/39144/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals

  • 135.4135.4_ZEEFS_13/step2_ZEEFS_13.log
  • 5.15.1_TTbarFS/step2_TTbarFS.log
  • 4.224.22_RunCosmics2011A/step5_RunCosmics2011A.log
Expand to see more relval errors ...

AddOn Tests

----- Begin Fatal Exception 29-Apr-2024 10:28:54 CEST-----------------------
An exception of category 'ConfigFileReadError' occurred while
   [0] Processing the python configuration file named /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02835/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_1_X_2024-04-28-2300/src/PhysicsTools/PatAlgos/test/IntegrationTest_cfg.py
Exception Message:
 unknown python problem occurred.
OSError: [Errno 5] Input/output error: '/cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44859/39144/CMSSW_14_1_X_2024-04-28-2300/python/Utilities/__init__.py'

At:
  <frozen importlib._bootstrap_external>(1039): get_data
  <frozen importlib._bootstrap_external>(982): get_code
  <frozen importlib._bootstrap_external>(846): exec_module
  <frozen importlib._bootstrap>(695): _load_unlocked
  <frozen importlib._bootstrap>(986): _find_and_load_unlocked
  <frozen importlib._bootstrap>(1007): _find_and_load
  <frozen importlib._bootstrap>(228): _call_with_frames_removed
  <frozen importlib._bootstrap>(972): _find_and_load_unlocked
  <frozen importlib._bootstrap>(1007): _find_and_load
  <frozen importlib._bootstrap>(228): _call_with_frames_removed
  <frozen importlib._bootstrap>(972): _find_and_load_unlocked
  <frozen importlib._bootstrap>(1007): _find_and_load
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02835/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_1_X_2024-04-28-2300/src/PhysicsTools/PatAlgos/python/tools/cmsswVersionTools.py(9): <module>
  <frozen importlib._bootstrap>(228): _call_with_frames_removed
  <frozen importlib._bootstrap_external>(850): exec_module
  <frozen importlib._bootstrap>(695): _load_unlocked
  <frozen importlib._bootstrap>(986): _find_and_load_unlocked
  <frozen importlib._bootstrap>(1007): _find_and_load
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02835/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_1_X_2024-04-28-2300/src/PhysicsTools/PatAlgos/python/patInputFiles_cff.py(2): <module>
  <frozen importlib._bootstrap>(228): _call_with_frames_removed
  <frozen importlib._bootstrap_external>(850): exec_module
  <frozen importlib._bootstrap>(695): _load_unlocked
  <frozen importlib._bootstrap>(986): _find_and_load_unlocked
  <frozen importlib._bootstrap>(1007): _find_and_load
  /cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02835/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_1_X_2024-04-28-2300/src/PhysicsTools/PatAlgos/test/IntegrationTest_cfg.py(40): <module>

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

cmsbuild avatar Apr 29 '24 10:04 cmsbuild

please test workflow 27634.0

fabiocos avatar Apr 29 '24 11:04 fabiocos

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f3bdfb/39149/summary.html COMMIT: 3bb166835691ae3db068fe2a342d12cd8981866f CMSSW: CMSSW_14_1_X_2024-04-29-1100/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/44859/39149/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline: 24834.78 step 2 The results for the comparisons for these workflows could be incomplete This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

  • You potentially removed 101 lines from the logs
  • Reco comparison results: 519 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3424349
  • DQMHistoTests: Total failures: 1729
  • DQMHistoTests: Total nulls: 21
  • DQMHistoTests: Total successes: 3422579
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -40.019 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 27634.0,... ): -5.717 KiB MTD/ETL
  • Checked 207 log files, 169 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

cmsbuild avatar Apr 29 '24 14:04 cmsbuild

@rdelliga , we may not have strong rules but in my experience more optimal following approach:

  1. do not use std::endl except if strongly need blank line in debug output.
  2. in class constructor it is enough to have edm::LogVerbatim without any protection. In any case it is not needed to have more than one protection of the debug printout.

civanch avatar May 01 '24 09:05 civanch

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44859/40164

  • This PR adds an extra 20KB to repository

  • There are other open Pull requests which might conflict with changes you have proposed:

    • File Validation/MtdValidation/plugins/EtlDigiHitsValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/EtlLocalRecoValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/EtlSimHitsValidation.cc modified in PR(s): #43928
    • File Validation/MtdValidation/plugins/MtdTracksValidation.cc modified in PR(s): #43928

cmsbuild avatar May 03 '24 13:05 cmsbuild

Pull request #44859 was updated. @subirsarkar, @mdhildreth, @mandrenguyen, @syuvivida, @antoniovagnerini, @jfernan2, @srimanob, @nothingface0, @rvenditti, @tjavaid, @cmsbuild, @civanch can you please check and sign again.

cmsbuild avatar May 03 '24 13:05 cmsbuild

please test with workflow 27634.0

fabiocos avatar May 03 '24 20:05 fabiocos

please test with workflow 27634.0

fabiocos avatar May 06 '24 08:05 fabiocos

@smuzaffar is there an issue with the test? code-checks look to have passed correctly...

fabiocos avatar May 06 '24 09:05 fabiocos

please test workflow 27634.0

@fabiocos , this is the right comment to run extra workflow :-) with is used for extra PRs to be included

smuzaffar avatar May 06 '24 09:05 smuzaffar

@smuzaffar oops sorry, my bad!

fabiocos avatar May 06 '24 09:05 fabiocos

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-f3bdfb/39250/summary.html COMMIT: f60ce34906a6ba1695dee32b1b7a85a76d4c5177 CMSSW: CMSSW_14_1_X_2024-05-05-2300/el8_amd64_gcc12 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44859/39250/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially removed 4 lines from the logs
  • Reco comparison results: 258 differences found in the comparisons
  • DQMHistoTests: Total files compared: 49
  • DQMHistoTests: Total histograms compared: 3429896
  • DQMHistoTests: Total failures: 3813
  • DQMHistoTests: Total nulls: 21
  • DQMHistoTests: Total successes: 3426042
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -40.019 KiB( 48 files compared)
  • DQMHistoSizes: changed ( 23234.0,... ): -5.717 KiB MTD/ETL
  • Checked 207 log files, 169 edm output root files, 49 DQM output files
  • TriggerResults: no differences found

cmsbuild avatar May 06 '24 13:05 cmsbuild

+1

civanch avatar May 06 '24 16:05 civanch

+1

jfernan2 avatar May 06 '24 17:05 jfernan2

@cms-sw/dqm-l2 any comment?

fabiocos avatar May 08 '24 06:05 fabiocos

+1

tjavaid avatar May 10 '24 08:05 tjavaid

@cms-sw/upgrade-l2 any doubt/comment about this PR?

fabiocos avatar May 16 '24 15:05 fabiocos

+Upgrade

srimanob avatar May 16 '24 15:05 srimanob

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

cmsbuild avatar May 16 '24 15:05 cmsbuild

+1

rappoccio avatar May 20 '24 14:05 rappoccio