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

Add PPS geometry and simulation tags to GTs #33404

Merged
merged 1 commit into from Apr 13, 2021

Conversation

malbouis
Copy link
Contributor

@malbouis malbouis commented Apr 12, 2021

PR description:

This PR updates includes three tags in the mcRun3 GTs for PPS so that PR #33250 and #33266 can move forward, as requested in the hn threads https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/4392.html and https://hypernews.cern.ch/HyperNews/CMS/get/calibrations/4393.html .

The tags are:

XMLFILE_CTPPS_Geometry_2021_113YV1 with label CTPPS
LHCInfo_2021_mc_v2
PPSOpticalFunctions_2021_mc_v1

The GTs diffs are:

2021 design
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/113X_mcRun3_2021_design_v6/113X_mcRun3_2021_design_v7

2021 realistic
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/113X_mcRun3_2021_realistic_v8/113X_mcRun3_2021_realistic_v9

2021 cosmics
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/113X_mcRun3_2021cosmics_realistic_deco_v7/113X_mcRun3_2021cosmics_realistic_deco_v8

2021 heavy ion
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/113X_mcRun3_2021_realistic_HI_v6/113X_mcRun3_2021_realistic_HI_v7

2023 realistic
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/113X_mcRun3_2023_realistic_v6/113X_mcRun3_2023_realistic_v7

2024 realistic
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/113X_mcRun3_2024_realistic_v6/113X_mcRun3_2024_realistic_v7

PR validation:

Tested with:
runTheMatrix.py -j8 -l 312.0,11634.0,11634.911,12434.0,limited --ibeos

if this PR is a backport please specify the original PR and why you need to backport that PR:

This PR is not a backport and no backport is needed.

@malbouis
Copy link
Contributor Author

test parameters:

  • workflow = 312.0,11634.0,11634.911,12434.0

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33404/22033

  • This PR adds an extra 12KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Configuration/AlCa

@malbouis, @yuanchao, @christopheralanwest, @cmsbuild, @tlampen, @pohsun, @francescobrivio can you please review it and eventually sign? Thanks.
@makortel, @Martin-Grunewald, @mmusich, @fabiocos, @tocheng 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

@malbouis
Copy link
Contributor Author

test parameters:

@christopheralanwest
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

-1

Failed Tests: AddOn
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e41c18/14166/summary.html
COMMIT: f35acf8
CMSSW: CMSSW_11_3_X_2021-04-11-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33404/14166/install.sh to create a dev area with all the needed externals and cmssw changes.

AddOn Tests

----- Begin Fatal Exception 12-Apr-2021 04:24:18 CEST-----------------------
An exception of category 'CTPPSGeometry' occurred while
   [0] Processing  Event run: 323775 lumi: 99 event: 111190435 stream: 3
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module CTPPSDiamondLocalTrackFitter/'ctppsDiamondLocalTracks'
   [4] Calling method for module CTPPSDiamondRecHitProducer/'ctppsDiamondRecHits'
Exception Message:
Not found detector with ID 2054160384, i.e. subDet=5 arm=0 station=1 rp=6
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 12-Apr-2021 04:24:14 CEST-----------------------
An exception of category 'CTPPSGeometry' occurred while
   [0] Processing  Event run: 325112 lumi: 6 event: 25520 stream: 3
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module CTPPSDiamondLocalTrackFitter/'ctppsDiamondLocalTracks'
   [4] Calling method for module CTPPSDiamondRecHitProducer/'ctppsDiamondRecHits'
Exception Message:
Not found detector with ID 2054160384, i.e. subDet=5 arm=0 station=1 rp=6
----- End Fatal Exception -------------------------------------------------
----- Begin Fatal Exception 12-Apr-2021 04:23:31 CEST-----------------------
An exception of category 'CTPPSGeometry' occurred while
   [0] Processing  Event run: 323775 lumi: 99 event: 111372621 stream: 0
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module CTPPSDiamondLocalTrackFitter/'ctppsDiamondLocalTracks'
   [4] Calling method for module CTPPSDiamondRecHitProducer/'ctppsDiamondRecHits'
Exception Message:
Not found detector with ID 2054160384, i.e. subDet=5 arm=0 station=1 rp=6
----- End Fatal Exception -------------------------------------------------
Expand to see more addon errors ...

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 65 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2865506
  • DQMHistoTests: Total failures: 31
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2865453
  • 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

The error is originated from https://github.com/cms-sw/cmssw/blob/master/Configuration/HLT/python/autoCondHLT.py#L56-L60

    'run3_data_FULL'         : ('run2_data_relval'        ,l1Menus['FULL']),
    'run3_data_GRun'         : ('run2_data_relval'        ,l1Menus['GRun']),
    'run3_data_HIon'         : ('run2_data_promptlike_hi' ,l1Menus['HIon']),
    'run3_data_PIon'         : ('run2_data_relval'        ,l1Menus['PIon']),
    'run3_data_PRef'         : ('run2_data_relval'        ,l1Menus['PRef']),

This PR updates the 2021 GTs, but it does not update the Run2 GT (as expected).
@cms-sw/hlt-l2 could you check whether we should update Configuration/HLT/python/autoCondHLT.py to run3_data_relval or apply any other fix?

@silviodonato
Copy link
Contributor

urgent
the deadline of 11_3_0_pre6 is tomorrow, and this PR is needed to include the PPS reco in Run3

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Apr 12, 2021

Hmm, there is no run3_data_relval available in AlCa's autoCond.py.
Could one update run2_data_relval ? Or make a cpy of run2_data_relval, call it run3_data_relval and add the missing items?

@silviodonato
Copy link
Contributor

Hmm, there is no run3_data_relval available in AlCa's autoCond.py.
Could one update run2_data_relval ? Or make a cpy of run2_data_relval, call it run3_data_relval and add the missing items?

The creation of a new run3_data_relval seems to be the best solution, @cms-sw/alca-l2 @malbouis what do you think?

@jan-kaspar
Copy link
Contributor

@malbouis Could that be there is a typo in the description and that the new PPS geometry is included, too?

@christopheralanwest
Copy link
Contributor

Hmm, there is no run3_data_relval available in AlCa's autoCond.py.
Could one update run2_data_relval ? Or make a cpy of run2_data_relval, call it run3_data_relval and add the missing items?

I don't think that would help, unless we introduced fake PPS conditions for Run 2. If I modify the failing addOnTests to use the Run2_2018 era, the addOnTests pass. I suspect that the Run3 era is pulling in some hardcoded conditions that are inconsistent with the actual Run 2 PPS geometry.

@jan-kaspar
Copy link
Contributor

Hmm, there is no run3_data_relval available in AlCa's autoCond.py.
Could one update run2_data_relval ? Or make a cpy of run2_data_relval, call it run3_data_relval and add the missing items?

I don't think that would help, unless we introduced fake PPS conditions for Run 2. If I modify the failing addOnTests to use the Run2_2018 era, the addOnTests pass. I suspect that the Run3 era is pulling in some hardcoded conditions that are inconsistent with the actual Run 2 PPS geometry.

For sure the current test setup is inconsistent: it declares Run3 but uses data from 2018 (Run 2). Thus a failure may be legitimate. On the other hand, the error complains about a RP which should be both in Run2 and Run3 geometries - thus I agree that there is yet something else going on... @malbouis Have you encountered something similar when running your tests yesterday?

@jan-kaspar
Copy link
Contributor

@malbouis @christopheralanwest what would be your recommended (easiest) way to test this PR in a private space (I'd like to put in some print outs to understand what's going on)? I see that there is this script:
/cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33404/14166/install.sh
But I am not sure whether I can tell the matrix test to include #33250 and #33266.

@silviodonato
Copy link
Contributor

@jan-kaspar I'm trying to update #33266 to include #33404 #33250. In this way you will be able to simply use git cms-merge-topic 33266 to get everything

@jan-kaspar
Copy link
Contributor

The error is originated from https://github.com/cms-sw/cmssw/blob/master/Configuration/HLT/python/autoCondHLT.py#L56-L60

    'run3_data_FULL'         : ('run2_data_relval'        ,l1Menus['FULL']),
    'run3_data_GRun'         : ('run2_data_relval'        ,l1Menus['GRun']),
    'run3_data_HIon'         : ('run2_data_promptlike_hi' ,l1Menus['HIon']),
    'run3_data_PIon'         : ('run2_data_relval'        ,l1Menus['PIon']),
    'run3_data_PRef'         : ('run2_data_relval'        ,l1Menus['PRef']),

This PR updates the 2021 GTs, but it does not update the Run2 GT (as expected).
@cms-sw/hlt-l2 could you check whether we should update Configuration/HLT/python/autoCondHLT.py to run3_data_relval or apply any other fix?

Sorry I am slow... Does it mean that now the tests run with Run2 geometry and Run3 era modifier? For PPS this might indeed lead to the observed problem - the PPS geometry data are interpreted differently depending on the Run2 or Run3 configuration. As explained here, there is now a bug which triggers Run2-like processing even if Run3 is declared. This bug is fixed in #33250, thus including it may unveil Run2/3 incompatibilities.

@jan-kaspar
Copy link
Contributor

@jan-kaspar I'm trying to update #33266 to include #33404 #33250. In this way you will be able to simply use git cms-merge-topic 33266 to get everything

Thanks!

@silviodonato
Copy link
Contributor

@jan-kaspar git cms-merge-topic silviodonato:reenablePPSrun3_new includes all three PRs. It is based on CMSSW_11_3_X_2021-04-12-1100, but unfortunately you need to compile 38 packages anyway :-(

@silviodonato
Copy link
Contributor

Let me write here an example of error

cmsDriver.py RelVal -s L1REPACK:Full --data --scenario=pp -n 10 --conditions auto:run3_hlt_PRef --relval 9000,50 --datatier RAW --customise=HLTrigger/Configuration/CustomConfigs.L1T --era Run3 --eventcontent RAW --fileout file:RelVal_Raw_PRef_DATA.root --filein /store/data/Run2018D/EphemeralHLTPhysics1/RAW/v1/000/323/775/00000/2E066536-5CF2-B340-A73B-209640F29FF6.root --customise_commands\=if\ hasattr\(process,\"simMuonGEMPadTask\"\)\:\ setattr\(process,\"simMuonGEMPadTask\",cms.Task\(\)\)

cmsDriver.py RelVal -s HLT:PRef,RAW2DIGI,L1Reco,RECO --data --scenario=pp -n 10 --conditions auto:run3_data_PRef --relval 9000,50 --datatier RAW-HLT-RECO --eventcontent FEVTDEBUGHLT --customise=HLTrigger/Configuration/CustomConfigs.L1THLT --era Run3 --processName=HLTRECO --filein file:RelVal_Raw_PRef_DATA.root --fileout file:RelVal_Raw_PRef_DATA_HLT_RECO.root

Please note that we are using --era Run3 with Run2018D RAW data.
@Martin-Grunewald perhaps we should use --era Run3 only in Run3 simulation or MWGR data.

@cms-sw/simulation-l2 @cms-sw/alca-l2 @jan-kaspar did we have the diamond detectors already in Run2? If not, we should find a way (eg. customization function) to disable them (if we need to keep --era Run3).

cc @cms-sw/reconstruction-l2

@Martin-Grunewald
Copy link
Contributor

Martin-Grunewald commented Apr 12, 2021

Sorry, but why is the PoolOutputModule triggering this error in the first place?

----- Begin Fatal Exception 12-Apr-2021 04:23:31 CEST-----------------------
An exception of category 'CTPPSGeometry' occurred while
   [0] Processing  Event run: 323775 lumi: 99 event: 111372621 stream: 0
   [1] Running path 'FEVTDEBUGHLToutput_step'
   [2] Prefetching for module PoolOutputModule/'FEVTDEBUGHLToutput'
   [3] Prefetching for module CTPPSDiamondLocalTrackFitter/'ctppsDiamondLocalTracks'
   [4] Calling method for module CTPPSDiamondRecHitProducer/'ctppsDiamondRecHits'
Exception Message:
Not found detector with ID 2054160384, i.e. subDet=5 arm=0 station=1 rp=6
----- End Fatal Exception -------------------------------------------------

There are no CTPPS modules running in the HLT...

@davidlange6
Copy link
Contributor

davidlange6 commented Apr 12, 2021 via email

@Martin-Grunewald
Copy link
Contributor

Ah right, RECO.

@christopheralanwest
Copy link
Contributor

The error is originated from https://github.com/cms-sw/cmssw/blob/master/Configuration/HLT/python/autoCondHLT.py#L56-L60

    'run3_data_FULL'         : ('run2_data_relval'        ,l1Menus['FULL']),
    'run3_data_GRun'         : ('run2_data_relval'        ,l1Menus['GRun']),
    'run3_data_HIon'         : ('run2_data_promptlike_hi' ,l1Menus['HIon']),
    'run3_data_PIon'         : ('run2_data_relval'        ,l1Menus['PIon']),
    'run3_data_PRef'         : ('run2_data_relval'        ,l1Menus['PRef']),

This PR updates the 2021 GTs, but it does not update the Run2 GT (as expected).
@cms-sw/hlt-l2 could you check whether we should update Configuration/HLT/python/autoCondHLT.py to run3_data_relval or apply any other fix?

Sorry I am slow... Does it mean that now the tests run with Run2 geometry and Run3 era modifier? For PPS this might indeed lead to the observed problem - the PPS geometry data are interpreted differently depending on the Run2 or Run3 configuration. As explained here, there is now a bug which triggers Run2-like processing even if Run3 is declared. This bug is fixed in #33250, thus including it may unveil Run2/3 incompatibilities.

Only these four addOnTests use the Run 2 geometry with the Run 3 era modifier. Does the bug fixed in #33250 correspond precisely to the feature needed to use a Run 3 HLT menu on Run 2 data? If I merge only PR #33404 and #33266 but not #33250, I am able to run addOnTests.py -t hlt_data_PRef,hlt_data_HIon,hlt_data_PIon,hlt_data_GRun.

The relval HLT GTs differ from the standard HLT GTs only by having a fixed trigger menu and snapshot time so I don't think that introducing a run3_data_relval key is appropriate at this point. I would rather not introduce a fictitious Run 2 geometry history into a GT for the purpose of handling a technical problem in a nonstandard configuration.

@jan-kaspar
Copy link
Contributor

Let me try addressing many of the above comments and questions.

Timing/diamond RPs existed in Run2 as well, but only 1 RP per LHC sector. In Run 3, there will be 2 RPs per arm. Because of this, we've generalised the way how the copy number from geometry XML files is translated into DetId. In order not break backward (Run2) compatibility, we've introduced isRun2 in the geometry builder:
https://github.com/cms-sw/cmssw/blob/master/Geometry/VeryForwardGeometryBuilder/src/DetGeomDesc.cc#L247
The idea was to have isRun2=false by default and set it to true only when a Run2 era is declared:
https://github.com/cms-sw/cmssw/blob/master/Geometry/VeryForwardGeometry/python/geometryRPFromDB_cfi.py#L19
Unfortunately, this logic is wrong with the current definition of ctpps_20XY flags - isRun2 is set to true even for Run3. That's why #33250 has been opened.

Now to the matrix test failures reported here. They occur for a "specific" configuration of Run2 GT with Run3 era activated. For PPS this should mean Run2-style copy numbers in the geometry XML data while Run3-like interpretation of them - it should lead to an error. So far the error was not visible because of the bug mentioned in the previous paragraph - isRun2 flag was set to true even for Run3 and thus things were fakely consistent. With #33250, the bug is fixed and the inconsistency of Run2 geometry with Run3 era processing occurs.

I hope this clarifies the situation ...

@silviodonato
Copy link
Contributor

Thank you @jan-kaspar for the clarification. @Martin-Grunewald maybe something like 440b14c might solve our problem. Perhaps it should be included in this function https://github.com/cms-sw/cmssw/blob/master/HLTrigger/Configuration/python/customizeHLTforCMSSW.py#L125

@jan-kaspar
Copy link
Contributor

Thank you @jan-kaspar for the clarification. @Martin-Grunewald maybe something like 440b14c might solve our problem. ...

Many thanks @silviodonato !

@silviodonato
Copy link
Contributor

This PR has been included in #33415

@cmsbuild cmsbuild merged commit 185cd0e into cms-sw:master Apr 13, 2021
@cvuosalo
Copy link
Contributor

@malbouis Please correct the description of this PR. It says:

The tags are:

PPSOpticalFunctions_2021_mc_v1 with label CTPPS
LHCInfo_2021_mc_v2
PPSOpticalFunctions_2021_mc_v1

"PPSOpticalFunctions_2021_mc_v1" is listed twice, while the actual tag name of "XMLFILE_CTPPS_Geometry_2021_113YV1" is not mentioned at all. PR descriptions are important reference information.

@malbouis
Copy link
Contributor Author

@malbouis Please correct the description of this PR. It says:

The tags are:

PPSOpticalFunctions_2021_mc_v1 with label CTPPS
LHCInfo_2021_mc_v2
PPSOpticalFunctions_2021_mc_v1

"PPSOpticalFunctions_2021_mc_v1" is listed twice, while the actual tag name of "XMLFILE_CTPPS_Geometry_2021_113YV1" is not mentioned at all. PR descriptions are important reference information.

@cvuosalo , done!

@cvuosalo
Copy link
Contributor

@malbouis Sorry, the PR description is now reversed. It should be:

XMLFILE_CTPPS_Geometry_2021_113YV1 with label CTPPS
LHCInfo_2021_mc_v2
PPSOpticalFunctions_2021_mc_v1

Thanks.

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

8 participants