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
Bugfix: Make Use of the 2019 Geometry from DB #24824
Conversation
please test |
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-24824/6774 |
The tests are being triggered in jenkins. |
A new Pull Request was created by @ianna (Ianna Osborne) for master. It involves the following packages: Configuration/Geometry @cmsbuild, @prebello, @Dr15Jones, @cvuosalo, @civanch, @ianna, @mdhildreth, @pgunnell, @kpedro88, @zhenhu can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
-1 Tested at: a7fe6dc You can see the results of the tests here: I found follow errors while testing this PR Failed tests: RelVals
When I ran the RelVals I found an error in the following worklfows: runTheMatrix-results/11624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+HARVESTFull_2019+ALCAFull_2019/step2_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+HARVESTFull_2019+ALCAFull_2019.log |
Comparison not run due to runTheMatrix errors (RelVals and Igprof tests were also skipped) |
@abdoulline @christopheralanwest regarding the error observed:
Can you check the 2019 GT to see if some condition has to be updated for HCAL trigger? |
@kpedro88 For concreteness, I will refer to the 103X_postLS2_realistic_v3 GT. Only the electronics map tag is updated from the 2018 tag so all conditions that reference upgraded HB channels need to be updated. Using the GT rather than hardcoded HCAL conditions is not supported at all for postLS2 scenarios. I think that the error that you quote arises because the HcalParameters tag specified by the GT (HCALParameters_Geometry_91YV3) is a 2018 scenario [1]. This in turn specifies the 2018 trigger mode [2] rather than the the 2019 trigger mode [3]. Therefore LUTs with 1024 rather than 2048 entries are used for HB trigger towers [4]. [1] https://twiki.cern.ch/twiki/bin/view/CMSPublic/SWGuideDataBaseGeometry cmssw/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc Lines 236 to 242 in 02d4198
[3] cmssw/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.cc Lines 243 to 247 in 02d4198
[4] cmssw/CalibCalorimetry/CaloTPG/src/CaloTPGTranscoderULUT.h Lines 48 to 54 in 02d4198
|
@christopheralanwest Whatever (except emap) conditions are in GT - they are "place holders". |
Just in case, I've dumped step 2 of the workflow in question 11624.0 as it's now in 10_3_0_pre5 (runs OK), so that Yana (@ianna) could look at the list of xml files (in the bottom of the dump) - if all these are included in the current PR? /afs/cern.ch/user/a/abdullin/public/xml_Geometry_2019_11624.0/step2_dump.txt |
@abdoulline - I've uploaded the 2019 HcalParameters tag (HCALParameters_Geometry_103YV5) and created a candidate GT queue: 103X_postLS2_design_Candidate_2018_10_09_09_19_28 Now the 11624.0 fails with a different message:
|
please test workflow 11624.0 |
The tests are being triggered in jenkins. |
Comparison job queued. |
Comparison is ready Comparison Summary:
|
The TrackerHitsV plots are still showing unexpected discrepancies. @perrotta, are you sure these came out okay in your private test? |
@kpedro88 : yes, I confirm it. The same plot as in https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_10_4_X_2018-10-23-2300+24824/29061/11624.0_TTbar_13+TTbar_13TeV_TuneCUETP8M1_2019_GenSimFull+DigiFull_2019+RecoFull_2019+HARVESTFull_2019+ALCAFull_2019/TrackerHitsV__TrackerHit_TOBHit_Localy_TOB_2.png (jenkins tests, run with 10 events) looks as follows in my 70 events sample: Just for test, I rerun locally with only 10 events, and I can reproduce exactly what was found by the automated tests: This looks like being a matter of statistics: not all TOBs (in this example) are hit with only 10, events, while all them are hit at least once in 70 events. In detail:
And evidently, the empty ones do not correspond with and without this PR. In the reference the empty TOBs (with 10 events) are: 3, 6, 10 |
Ah, I see: it is just a very specific plot, so statistical fluctuations can leave it empty. Thanks for the followup, I am satisfied. |
+upgrade |
+1 |
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. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
There are issues in the local relval tests, for example: