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

PPS: add Run-3 reco, add geometry to GTs, update era modifier #33415

Merged
merged 12 commits into from Apr 13, 2021

Conversation

silviodonato
Copy link
Contributor

@silviodonato silviodonato commented Apr 12, 2021

This is the combination of three PRs

More details can be found in each PR.

This PR includes also a fix in the HLT AddOn test and cmsDriver in order to be able to run on Run-2 data using --procModifier Run3. This fix affects:

HLTrigger/Configuration/test/cmsDriver.csh
HLTrigger/Configuration/python/CustomConfigs.py
Configuration/HLT/python/addOnTestsHLT.py

@silviodonato
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33415/22061

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @silviodonato (Silvio Donato) for master.

It involves the following packages:

Configuration/AlCa
Configuration/Eras
Configuration/EventContent
Configuration/StandardSequences
DQM/CTPPS
HLTrigger/Configuration
PhysicsTools/PatAlgos
RecoPPS/Configuration
RecoPPS/Local
RecoPPS/ProtonReconstruction

@andrius-k, @Martin-Grunewald, @tlampen, @pohsun, @perrotta, @yuanchao, @silviodonato, @ErnestaP, @ahmad3213, @fwyzard, @davidlange6, @jfernan2, @slava77, @jpata, @qliphy, @fabiocos, @francescobrivio, @malbouis, @kmaeshima, @christopheralanwest, @franzoni, @rvenditti can you please review it and eventually sign? Thanks.
@rappoccio, @gouskos, @felicepantaleo, @forthommel, @hatakeyamak, @emilbols, @Martin-Grunewald, @mbluj, @ahinzmann, @seemasharmafnal, @mmarionncern, @makortel, @dgulhan, @jdolen, @slomeo, @ferencek, @GiacomoSguazzoni, @rovere, @VinInn, @jdamgov, @nhanvtran, @gkasieczka, @tocheng, @schoef, @mmusich, @mtosi, @fabiocos, @clelange, @jan-kaspar, @swozniewski, @JyothsnaKomaragiri, @lecriste, @gpetruc, @mariadalfonso, @andrzejnovak this is something you requested to watch as well.
@silviodonato, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

@silviodonato
Copy link
Contributor Author

please test workflow 11725.0,11925.0

@cmsbuild
Copy link
Contributor

-1

Failed Tests: Build Python3
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-914931/14200/summary.html
COMMIT: 440b14c
CMSSW: CMSSW_11_3_X_2021-04-12-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33415/14200/install.sh to create a dev area with all the needed externals and cmssw changes.

Build

I found compilation warning when building: See details on the summary page.

#Python3

I found errors:

*** Error compiling 'src/HLTrigger/Configuration/python/CustomConfigs.py'...

@@ -200,3 +200,8 @@ def L1XML(process,xmlFile=None):
process.ESPreferL1TXML = cms.ESPrefer("L1TUtmTriggerMenuESProducer","L1TriggerMenu")

return process

def CTPPSRun2Geometry(process):
if hasattr(process,'ctppsGeometryESModule')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if hasattr(process,'ctppsGeometryESModule')
if hasattr(process,'ctppsGeometryESModule'):

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is also necessary to return process as well as actually use the customization. Using the commit at 1364f26 on top of this PR allows the following to complete without errors in a local test:

  • runTheMatrix.py -l 11725.0,11925.0,limited --ibeos
  • addOnTests -j 8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thansk @forthommel @christopheralanwest .
@Martin-Grunewald is Configuration/HLT/python/addOnTestsHLT.py automatically generated from HLTrigger/Configuration/test/cmsDriver.csh ? Do we need to change both manually?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are separate, both need the change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I've added 1364f26

@Martin-Grunewald
Copy link
Contributor

+1

@perrotta
Copy link
Contributor

+1

@silviodonato
Copy link
Contributor Author

test parameters:
workflow = 11725.0,11925.0

@silviodonato
Copy link
Contributor Author

please test
this is a further technical check by using the workflow specific for testing PPS
@cms-sw/dqm-l2 this additional test will not change the comparison result

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-914931/14213/summary.html
COMMIT: d7a53f4
CMSSW: CMSSW_11_3_X_2021-04-12-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33415/14213/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-914931/11725.0_GluGluTo2Jets_14TeV+2021+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+Digi+Reco+HARVEST+ALCA
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-914931/11925.0_GluGluTo2Jets_14TeV+2021PU+GluGluTo2Jets_M_300_2000_14TeV_Exhume_GenSim+DigiPU+RecoPU+HARVESTPU+Nano

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 40 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2865526
  • DQMHistoTests: Total failures: 20
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2865484
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@silviodonato
Copy link
Contributor Author

@jfernan2 the DQM part of the PR is basically renaming ctpps_2016 to ctpps
in

DQM/CTPPS/python/ctppsDQM_cff.py
DQM/CTPPS/test/all_ctpps_dqm_test_from_aod_cfg.py
DQM/CTPPS/test/all_ctpps_dqm_test_from_dat_cfg.py 
DQM/CTPPS/test/all_ctpps_dqm_test_from_raw_cfg.py

@jfernan2
Copy link
Contributor

+1

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

@silviodonato
Copy link
Contributor Author

+1

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

9 participants