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

Update HLT/HLT relval/Express/Prompt GTs #33206

Merged
merged 6 commits into from
Mar 24, 2021

Conversation

christopheralanwest
Copy link
Contributor

PR description:

This PR makes several changes to the online GTs to bring several up-to-date and simplify maintenance.

  • The run1_hlt key is replaced by a run1_hlt_relval key as the only usage of this key is in Run 1 addOnTests in which the L1T menu is intended to be fixed, as in relvals
  • The run2_hlt key has been removed, as it is not used anywhere in CMSSW
  • A new HLT autoCond key, run3_hlt has been introduced and is intended to be kept up-to-date with the actual HLT GT. This key is used by the DQM/Integration package and is also used in the legacy autoCond['hltonline'] key (intended to represent the online HLT). There are no other uses of this key in CMSSW.
  • The GT represented by the value of key run2_hlt_relval has been brought up-to-date with respect to the current online GT. It now is equivalent to the online HLT key, aside from a fixed snapshot time and L1T trigger menu
  • The run2_hlt_relval_hi key was superfluous. It differs from the run2_hlt_relval key only by the L1 trigger menu. Since only usage was in autoCondHLT, which overrides the trigger menu, I have modified the relevant entry to use the run2_hlt_relval key and removed the run2_hlt_relval_hi key.
  • run2_hlt_hi is used only by workflow 140.55, representing 2015 HI data and requires a special tag SiStripFullPedestals_GR10_v1_hlt and a special L1T menu. Since this is a very special scenario, this GT has been represented by a symbolic GT that implements these changes on top of the run2_hlt_relval key.

During the course of preparing this PR, I discovered that an the PFCalibrationRcd with label "HLT" was incorrect in the HLT, Express and Prompt GTs. The corrected tag is now used.

The GT diffs are as follows:

Run 2 HLT RelVals
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/112X_dataRun2_HLT_relval_v4/112X_dataRun2_HLT_relval_v5

Run 3 data (express)
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/112X_dataRun3_Express_v3/112X_dataRun3_Express_v4

Run 3 data (prompt)
https://cms-conddb.cern.ch/cmsDbBrowser/diff/Prod/gts/112X_dataRun3_Prompt_v3/112X_dataRun3_Prompt_v4

The large number of EGM calibration records that are present in old Run 2 HLT relvals are not consumed (or else a crash would result in various tests).

Attn: @mmusich @Martin-Grunewald

PR validation:

This PR is mainly technical and has been validated with:

  • runTheMatrix.py -l 138.1,138.2,140.55 --ibeos
  • addOnTests.py -j 8
  • scram b runtests

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

Since this PR affects GTs in the 112X release, a backport to 11_2_X is intended.

@christopheralanwest
Copy link
Contributor Author

test parameters

  • workflow = 138.1,138.2,140.55

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33206/21640

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

Configuration/AlCa
Configuration/HLT
DQM/Integration

@malbouis, @andrius-k, @yuanchao, @kmaeshima, @fwyzard, @christopheralanwest, @ErnestaP, @ahmad3213, @Martin-Grunewald, @cmsbuild, @jfernan2, @tlampen, @pohsun, @rvenditti, @francescobrivio can you please review it and eventually sign? Thanks.
@batinkov, @battibass, @makortel, @tocheng, @Martin-Grunewald, @mmusich, @threus, @fabiocos 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

@christopheralanwest
Copy link
Contributor Author

please test

Copy link
Contributor

@mmusich mmusich left a comment

Choose a reason for hiding this comment

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

Thanks for the update. Some observations / questions below:

# GlobalTag for Run3 HLT: it points to the online GT
'run3_hlt' : '112X_dataRun3_HLT_v2',
# GlobalTag with fixed snapshot time for Run1 HLT RelVals: customizations to run with fixed L1 Menu
'run1_hlt_relval' : '112X_dataRun2_HLT_relval_v5',
Copy link
Contributor

Choose a reason for hiding this comment

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

Since it points anyway to the same GT as the Run2 key, what about enforcing the identity, by using the compatibility dictionary below?
Same thing is true for the run1_data and run2_data keys.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The equality of each pair is now guaranteed by commit 988b4e0. These keys cannot be added with the compatibility keys as the run1_hlt_relval key is needed by autoCondHLT earlier in the file.

Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't a re-ordering be enough here?

diff --git a/Configuration/AlCa/python/autoCond.py b/Configuration/AlCa/python/autoCond.py
index f4f08d09b5c..c94856c1d01 100644
--- a/Configuration/AlCa/python/autoCond.py
+++ b/Configuration/AlCa/python/autoCond.py
@@ -84,29 +84,11 @@ aliases = {
     'BASEGT' : 'BASE1_V1|BASE2_V1'
 }
 
-### Run 1 data GTs ###
+### OLD KEYS ### kept for backward compatibility
     # GlobalTag with fixed snapshot time for Run1 HLT RelVals: customizations to run with fixed L1 Menu
 autoCond['run1_hlt_relval']  = autoCond['run2_hlt_relval']
     # GlobalTag for Run1 data reprocessing
 autoCond['run1_data']        = autoCond['run2_data']
-
-# dedicated GlobalTags for HLT
-from Configuration.HLT.autoCondHLT import autoCondHLT
-autoCond = autoCondHLT(autoCond)
-
-# dedicated GlobalTags for phase-2 (specializing conditions for each geometry)
-from Configuration.AlCa.autoCondPhase2 import autoCondPhase2
-autoCond = autoCondPhase2(autoCond)
-
-# special cases modifier for autoCond GTs
-from Configuration.AlCa.autoCondModifiers import autoCond0T
-autoCond = autoCond0T(autoCond)
-
-# special GT for 2015 HLT HI run
-from Configuration.AlCa.autoCondModifiers import autoCondHLTHI
-autoCond = autoCondHLTHI(autoCond)
-
-### OLD KEYS ### kept for backward compatibility
     # GlobalTag for MC production with perfectly aligned and calibrated detector
 autoCond['mc']               = ( autoCond['run1_design'] )
     # GlobalTag for MC production with realistic alignment and calibrations
@@ -125,3 +107,19 @@ autoCond['upgradePLS150ns']  = ( autoCond['run2_mc_50ns'] )
 autoCond['upgrade2017']      = ( autoCond['phase1_2017_design'] )
 autoCond['upgrade2021']      = ( autoCond['phase1_2021_design'] )
 autoCond['upgradePLS3']      = ( autoCond['phase2_realistic'] )
+
+# dedicated GlobalTags for HLT
+from Configuration.HLT.autoCondHLT import autoCondHLT
+autoCond = autoCondHLT(autoCond)
+
+# dedicated GlobalTags for phase-2 (specializing conditions for each geometry)
+from Configuration.AlCa.autoCondPhase2 import autoCondPhase2
+autoCond = autoCondPhase2(autoCond)
+
+# special cases modifier for autoCond GTs
+from Configuration.AlCa.autoCondModifiers import autoCond0T
+autoCond = autoCond0T(autoCond)
+
+# special GT for 2015 HLT HI run
+from Configuration.AlCa.autoCondModifiers import autoCondHLTHI
+autoCond = autoCondHLTHI(autoCond)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer the current ordering as the keys run1_hlt_relval and run1_data are standard autoCond keys, even though they each are fixed to have the same value as another key. I would rather keep the deprecated keys separate at the end of the file.

# GlobalTag for Run3 data relvals
'run3_data_promptlike' : '112X_dataRun3_Prompt_v3',
'run3_data_promptlike' : '112X_dataRun3_Prompt_v4',
Copy link
Contributor

@mmusich mmusich Mar 17, 2021

Choose a reason for hiding this comment

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

Since this is a real infinite time snapshot GlobalTag suitable for Prompt Reco (with all the connected risks associated to it) what's the point of calling it 'promptlike'? Would not it be better to call it run3_prompt? Also is the plan during pp operations to keep updating it as frequently as Tier-0 does?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the key name to run3_data_prompt in commit f9b641c. Once an offline Run 3 GT is available, workflows that use run3_data_prompt would be converted to a run3_data key. Of course, it would be possible to create an offline GT for Run 3. But since that GT would currently have no other uses, I think it's preferable to exercise a GT that is actually used.

As long as there are no tags contained in the prompt GTs with synchronizations not allowed by the prompt queue, I think that the synchronization policy would be sufficient to ensure reproducibility. Is there another potential way to violate reproducibility that I am not considering?

Copy link
Contributor

Choose a reason for hiding this comment

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

As long as there are no tags contained in the prompt GTs with synchronizations not allowed by the prompt queue, I think that the synchronization policy would be sufficient to ensure reproducibility. Is there another potential way to violate reproducibility that I am not considering?

I am mostly concerned with private and "semi-official" (such as AlCaRelVals for conditions validation) usage of the autoCond keys. As soon as we have 24/7 operations someone might be tempted to use this key to process fresh data, possibly even earlier that PromptReco is released for that run. So in principle you might get different results with that recipe depending if you run before or after than the PCL and O2Os for that run have completed. For RelVals and unit tests using a fixed run (from the relatively distant past) the prompt synchronization will enforce reproducibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With a fixed snapshot time, analysis of runs taken after the snapshot time could receive incorrect, old conditions. If someone tries to process data before the PromptReco is released for that run, they will run into problems independent of whether the prompt GT is used in autoCond or not. I will let the other AlCa conveners decide what they prefer to do here, as they will be maintaining these GTs in the future, not me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe for the time being we could choose to keep this GT in autoCond as @christopheralanwest suggests and reassess it in the future, in case we think it is necessary.


for key,val in six.iteritems(autoCond):
if key == 'run2_hlt_relval': # modification of HLT relval GT
GlobalTagsHLTHI['run2_hlt_hi'] = (autoCond[key], FullPedestalsForHLTHI, MenuForHLTHI)
Copy link
Contributor

Choose a reason for hiding this comment

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

@icali @CesarBernardes please take note of this. This GlobalTag, suitable for Hybrid ZeroSuppression emulation on 2015 Heavy Ion ViriginRaw data has become symbolic. This means it's fine for local tests and RelVals but not e.g. for central samples production. Please comment if this might constitue a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course, if it is desired to reconstruct 2015 HI data in 11_2_X or later, AlCa could trivially create a regular, non-symbolic GT. Our assumption is that this would be needed rarely enough that it is simpler to maintain the GT as a symbolic GT based on the run2_hlt_relval GT.

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-aebe2a/13578/summary.html
COMMIT: a303007
CMSSW: CMSSW_11_3_X_2021-03-17-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33206/13578/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-aebe2a/138.1_RunCosmics2020+RunCosmics2020+RECOCOSDRUN3+ALCACOSDRUN3+HARVESTDCRUN3
  • /data/cmsbld/jenkins/workspace/compare-root-files-short-matrix/data/PR-aebe2a/138.2_RunCosmics2020+RunCosmics2020+RECOCOSDEXPRUN3+ALCACOSDEXPRUN3+HARVESTDCEXPRUN3

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2639881
  • DQMHistoTests: Total failures: 7
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2639852
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 36 files compared)
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@malbouis
Copy link
Contributor

+1

For the online GTs the snapshot time is as expected and the right PFCalibration tag is used, PFCalibration_HLT_2017_25ns_Spring17_v3_hlt.
The relval tags have also been updated consistently.

@Martin-Grunewald
Copy link
Contributor

+1

@jfernan2
Copy link
Contributor

+1

@srimanob
Copy link
Contributor

+Upgrade

This PR is to organize GTs.

@chayanit
Copy link

+1

@ggovi
Copy link
Contributor

ggovi commented Mar 24, 2021

+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)

@qliphy
Copy link
Contributor

qliphy commented Mar 24, 2021

+1

@cmsbuild cmsbuild merged commit 1f939c8 into cms-sw:master Mar 24, 2021
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.

10 participants