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

Phase 2 HLT menu targetting 7.5×10³⁴ cm⁻²s⁻¹ [11.1.x] #32903

Merged
merged 11 commits into from Sep 29, 2021

Conversation

fwyzard
Copy link
Contributor

@fwyzard fwyzard commented Feb 12, 2021

PR description:

Implement the Phase-2 HLT menu used for the HLT TDR.

HLT_75e33_cfg.py implements a self-contained configuration:

  • selects the required input collections for running on a re-emulated L1 sample;
  • uses the global tag 111X_mcRun4_realistic_T15_v5;
  • configures the MessageLogger, DQMStore, FastTimerService and ThroughputService.

HLT_75e33_cff.py implements a configuration fragment for use wih cmsDriver. To avoid redefinition errors, most of the L1T modules and the offline beamspot module are dropped from this configuration.

The HLT configuration is based on

and implement the baseline Phase-2 configuration for Jets/MET, b-tagging, Muons and E/Gamma.
Taus are not included.

It includes "open" or "MC" HLT paths for running the reconstruction without any (pre)selection applied, and a partial re-emulation of the Level-1 Trigger, which simulates a simplified L1T menu using EDFilters and Paths.

For Jets/MET and B-tagging, the exact results obtained for the TDR should be reproducible with the commit 19f3c18 (and likely also with 93a4a57, which is just a technical fix), which still use "offline" muons for the Particle Flow reconstruction.
The following commits implement the use of HLT muons in the Particle Flow reconstruction, affecting the Jets/MET (and likely B-tagging) performance in a non-negligible way; for more details see this presentation.

For Muons, E/Gamma, and overall timing measurements, the exact results obtained in the TDR should be reproducible using the full PR.

PR validation:

The configuration in HLTrigger/Configuration/python/HLT_75e33_cfg.py is as close as it can be to the one used for the HLT TDR.

The configuration in HLTrigger/Configuration/python/HLT_75e33_cff.py is almost identical, and was validated by running:

$ cmsDriver.py step2  --conditions auto:phase2_realistic_T15 -s HLT:75e33 --geometry Extended2026D49 --era Phase2C9 --eventcontent FEVTDEBUGHLT --datatier GEN-SIM-DIGI-RAW -n 10 --filein /store/mc/Phase2HLTTDRSummer20ReRECOMiniAOD/MinBias_TuneCP5_14TeV-pythia8/SKIM/PU200_111X_mcRun4_realistic_T15_v1-v1/000001/2cf57ee0-f543-4718-a2f1-db8b9baeac87.root --processName=HLTX -n 20 --nThreads 8
...
(skip various initialisation messges)
Begin processing the 1st record. Run 1, Event 162234, LumiSection 917 on stream 5 at 19-Sep-2021 11:06:02.253 CEST
%MSG-w TwoTrackMinimumDistance:   InclusiveCandidateVertexFinder:hltDeepInclusiveVertexFinderPF  19-Sep-2021 11:06:27 CEST Run: 1 Event: 162234
comparing track with itself!
%MSG
...
Begin processing the 14th record. Run 1, Event 519298, LumiSection 2934 on stream 6 at 19-Sep-2021 11:08:53.867 CEST
Begin processing the 15th record. Run 1, Event 519318, LumiSection 2934 on stream 7 at 19-Sep-2021 11:08:53.881 CEST
Begin processing the 16th record. Run 1, Event 169611, LumiSection 959 on stream 1 at 19-Sep-2021 11:09:18.739 CEST
Begin processing the 17th record. Run 1, Event 127902, LumiSection 723 on stream 7 at 19-Sep-2021 11:09:34.448 CEST
Begin processing the 18th record. Run 1, Event 127910, LumiSection 723 on stream 6 at 19-Sep-2021 11:09:34.466 CEST
Begin processing the 19th record. Run 1, Event 127969, LumiSection 723 on stream 3 at 19-Sep-2021 11:09:34.480 CEST
Begin processing the 20th record. Run 1, Event 526297, LumiSection 2974 on stream 0 at 19-Sep-2021 11:10:09.478 CEST
19-Sep-2021 11:10:24 CEST  Closed file file:///gpu_data/store/mc/Phase2HLTTDRSummer20ReRECOMiniAOD/MinBias_TuneCP5_14TeV-pythia8/SKIM/PU200_111X_mcRun4_realistic_T15_v1-v1/000001/2cf57ee0-f543-4718-a2f1-db8b9baeac87.root

=============================================

MessageLogger Summary

 type     category        sev    module        subroutine        count    total
 ---- -------------------- -- ---------------- ----------------  -----    -----
    1 TwoTrackMinimumDista -w InclusiveCandida                      72       72
    2 fileAction           -s file_close                             1        1
    3 fileAction           -s file_open                              2        2

 type    category    Examples: run/evt        run/evt          run/evt
 ---- -------------------- ---------------- ---------------- ----------------
    1 TwoTrackMinimumDistance 1/162234         1/162234         1/512433
    2 fileAction           PostGlobalEndRun                  
    3 fileAction           pre-events       pre-events       

Severity    # Occurrences   Total Occurrences
--------    -------------   -----------------
Warning                72                  72
System                  3                   3

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @fwyzard (Andrea Bocci) for CMSSW_11_1_X.

It involves the following packages:

HLTrigger/Phase2

The following packages do not have a category, yet:

HLTrigger/Phase2
Please create a PR for https://github.com/cms-sw/cms-bot/blob/master/categories_map.py to assign category

@cmsbuild can you please review it and eventually sign? Thanks.
@Martin-Grunewald 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

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 12, 2021

@Sam-Harper @trtomei FYI

@missirol
Copy link
Contributor

missirol commented Feb 13, 2021

I'm copying here for reference a quick exchange with Andrea, regarding a few small follow-ups to this PR, to be done in the next days.


(quoted text from me, answers from Andrea)

  • process.options contains various strings that reference RECO2, so they’d probably have no effect in this config; should we change this to HLTX, or remove them altogether (I don’t know what the canDeleteEarly option actually does)?

Yes, this needs to be cleaned.

  • a TFileService module slipped in when the BTV part was integrated; I guess it is better to remove it.

Yes.

  • I made a mistake, because I effectively gave you a path with “PFMET+MHT”, but in the TDR we plan to use “PFMET Type1” rather than (Raw) PFMET. How do you prefer to proceed in this case? I can update the recipe in my user package (and you re-export), or I can make a PR to your branch with the necessary changes (it is a relatively simple change, see here).

Make a PR to my branch, please.

  • in HLT_75e33_cfg.py the BTV paths are missing (while they are present in HLT_75e33.py); you probably did it intentionally, though, because if I include them I get an error (some cms-data files called by BTV are missing).

No, I just forgot to include it in the update :-(

  • right now we have a MC path (called “MC_JME”) which is not really a path, but just a sequence of a lot of different collections, many of which we don’t really use (e.g. AK4PFJets, AK4PFCHSJets, and AK8* jets). I had removed it from the JME config, but it somehow reappeared when you integrated the BTV part. What do you prefer to do with it? I would say it is not really needed (we could at least remove the collections that are not used in any of the actual triggers).

I would keep it and make it optional, so people can use it to make ntuples without a priori cuts.
Of course, cleaning it up is appreciated :-)

  • one thing that we will have to change at some point is the HLT thresholds (for JME), because right now they are simply taken from 2018 for that config (e.g. Jet500, HT1050, MET120_MHT120).

OK, sure.

@Sam-Harper
Copy link
Contributor

Thanks Andrea.

I do have a small question on technical nature of the menu structure. The issue is that the menu loads manually in all objects. For E/gamma we took a different approach
https://gitlab.cern.ch/sharper/EgHLTPhase2/-/blob/master/Menus/egammaBasicMenu.py

which is where the menu just loads the path and the path is assumed to be able to load everything it needs in. Is there an issue with doing it this way?

For esproducers, esources and psets, this is of course difficult to do (although thinking about it there is no reason why modules requiring external psets can not just import them so that when they imported the pset is there) and I default to loading all available. While I admit this is a not a great solution, having them all loaded in the menu one by one also doesnt seem great it will likely become tedious to maintain and the ultimate effect will likely be just loading everything anyways.

@fwyzard
Copy link
Contributor Author

fwyzard commented Feb 13, 2021 via email

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2021

Pull request #32903 was updated. @cmsbuild can you please check and sign again.

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 9, 2021

@missirol

  • process.options contains various strings that reference RECO2, so they’d probably have no effect in this config; should we change this to HLTX, or remove them altogether (I don’t know what the canDeleteEarly option actually does)?

canDeleteEarly tells the framework that there are no dependencies (edm::Ref, edm::Ptr, raw pointers, etc.) on these collections, and that they can be deleted as soon as there are no other modules that depend on it that need to be run.

It'll probably make sense to drop these entirely, but for the time being I've changed them to HLTX.

  • a TFileService module slipped in when the BTV part was integrated; I guess it is better to remove it.

Done.

  • I made a mistake, because I effectively gave you a path with “PFMET+MHT”, but in the TDR we plan to use “PFMET Type1” rather than (Raw) PFMET. How do you prefer to proceed in this case? I can update the recipe in my user package (and you re-export), or I can make a PR to your branch with the necessary changes (it is a relatively simple change, see here).

Thanks for having updated your setup - these changes should now be included here as well.

  • in HLT_75e33_cfg.py the BTV paths are missing (while they are present in HLT_75e33.py); you probably did it intentionally, though, because if I include them I get an error (some cms-data files called by BTV are missing).

I've regenerated the full dump now.

  • one thing that we will have to change at some point is the HLT thresholds (for JME), because right now they are simply taken from 2018 for that config (e.g. Jet500, HT1050, MET120_MHT120).

Let me know if you have an update on this.

@missirol
Copy link
Contributor

missirol commented Mar 9, 2021

Thanks, @fwyzard.

  • one thing that we will have to change at some point is the HLT thresholds (for JME), because right now they are simply taken from 2018 for that config (e.g. Jet500, HT1050, MET120_MHT120).

Let me know if you have an update on this.

There is no particular update on this; these HLT thresholds are effectively placeholders at the moment (I could replace them with the thresholds reported at the AR, though). My thinking was to update them once we have the final rate estimates that will be used in the TDR (but let me know if you have something different in mind).

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2021

Pull request #32903 was updated. @cmsbuild can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2021

Pull request #32903 was updated. @cmsbuild can you please check and sign again.

1 similar comment
@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2021

Pull request #32903 was updated. @cmsbuild can you please check and sign again.

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 9, 2021

Pull request #32903 was updated. @cmsbuild can you please check and sign again.

@fwyzard
Copy link
Contributor Author

fwyzard commented Mar 9, 2021

@Martin-Grunewald so far I'm using HLTrigger/Phase2 for the Phase 2 HLT configurations; do you prefer that I put them under HLTrigger/Configuration ?

The structure could be (once I actually make both cff and cfg files) something like

HLTrigger/Configuration/python/HLT_5p0e34/modules/...
HLTrigger/Configuration/python/HLT_5p0e34/paths/...
HLTrigger/Configuration/python/HLT_5p0e34/...
HLTrigger/Configuration/python/HLT_5p0e34_cff.py
HLTrigger/Configuration/python/HLT_7p5e34/modules/...
HLTrigger/Configuration/python/HLT_7p5e34/paths/...
HLTrigger/Configuration/python/HLT_7p5e34/...
HLTrigger/Configuration/python/HLT_7p5e34_cff.py
HLTrigger/Configuration/test/HLT_5p0e34_cfg.py
HLTrigger/Configuration/test/HLT_7p5e34_cfg.py

@Martin-Grunewald
Copy link
Contributor

@fwyzard
Indeed I prefer putting them under HLTrigger/Configuration with the directory structure you indicate above!

@cmsbuild
Copy link
Contributor

Pull request #32903 was updated. @Martin-Grunewald, @cmsbuild, @fwyzard can you please check and sign again.

@cmsbuild
Copy link
Contributor

Pull request #32903 was updated. @cmsbuild, @jordan-martins, @missirol, @bbilin, @wajidalikhan, @Martin-Grunewald, @AdrianoDee, @srimanob, @kskovpen can you please check and sign again.

@fwyzard
Copy link
Contributor Author

fwyzard commented Sep 21, 2021

please test

@missirol
Copy link
Contributor

(Just a note for completeness)

As briefly discussed with Andrea offline, the previous differences in wf 20434.0 (which uses D41) might have been related to the HLT overwriting some (ES) modules with D49 settings, and these modules being used in DIGI sequences (since the DIGI and HLT steps are run together in that wf).

Now the use of the HLT Phase-2 menu is being restricted to the wf that uses the D49 geometry (i.e. the only geometry for which the Phase-2 HLT guarantees meaningful outputs).

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-741dce/18793/summary.html
COMMIT: 205db68
CMSSW: CMSSW_11_1_X_2021-09-19-0000/slc7_amd64_gcc820
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/32903/18793/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

The workflows 140.53 have different files in step1_dasquery.log than the ones found in the baseline. You may want to check and retrigger the tests if necessary. You can check it in the "files" directory in the results of the comparisons

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 1275 differences found in the comparisons
  • DQMHistoTests: Total files compared: 36
  • DQMHistoTests: Total histograms compared: 2784828
  • DQMHistoTests: Total failures: 2030
  • DQMHistoTests: Total nulls: 44
  • DQMHistoTests: Total successes: 2782704
  • DQMHistoTests: Total skipped: 50
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -45.703 KiB( 35 files compared)
  • DQMHistoSizes: changed ( 140.53 ): -44.531 KiB Hcal/DigiRunHarvesting
  • DQMHistoSizes: changed ( 140.53 ): -1.172 KiB RPC/DCSInfo
  • Checked 152 log files, 31 edm output root files, 36 DQM output files
  • TriggerResults: no differences found

@Martin-Grunewald
Copy link
Contributor

+1

@srimanob
Copy link
Contributor

@missirol
Copy link
Contributor

Hi @srimanob

  • for wf 23234.0, see this comment from this PR;

  • in short, it'd affect any non-D49 wf. The 'overwriting' is what I guess is happening; if it happens, it could explain differences in outputs for non-D49 scenarios where DIGI and HLT are run in the same step; the overwriting itself would also occur in D49 wfs, but it would lead to no changes in the configuration b/c the HLT modules are tailored towards D49.

@srimanob
Copy link
Contributor

+Upgrade

This PR implements Phase-2 HLT menu to 11_1 release. Currently, we should focus on D49 result which give meaningful of HLT result.

@kskovpen
Copy link
Contributor

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next CMSSW_11_1_X IBs (tests are also fine) and once validation in the development release cycle CMSSW_12_1_X is complete. This pull request will now be reviewed by the release team before it's merged. @perrotta, @dpiparo, @qliphy (and backports should be raised in the release meeting by the corresponding L2)

@qliphy
Copy link
Contributor

qliphy commented Sep 29, 2021

+1

@cmsbuild cmsbuild merged commit e7b464b into cms-sw:CMSSW_11_1_X Sep 29, 2021
@fwyzard fwyzard deleted the HLT_Phase2_menu branch October 29, 2022 08:40
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

10 participants