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

EMTF Primitive Conversion LUT update for 2023 #42198

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

eyigitba
Copy link
Contributor

@eyigitba eyigitba commented Jul 6, 2023

PR description:

This PR adds options to use the new primitive conversion LUTs in the EMTF emulator. The LUTs were deployed at P5 on July 1st, but they are not used in the emulator yet.

This PR needs the LUTs in cms-data/L1Trigger-L1TMuon#25 to work.

We see improvement in EMTF performance at P5 with these new LUTs, so changes to muon efficiencies etc are expected when testing the re-emulation workflows from recent runs.

This PR will be backported to 13_1_X and 13_0_X.

PR validation:

Validated by comparing unpacked and re-emulated collections from recent runs. Also ran runTheMatrix.py -l limited -i all --ibeos and no failures were observed.

If this PR is a backport please specify the original PR and why you need to backport that PR. If this PR will be backported please specify to which release cycle the backport is meant for:

N/A

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2023

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-42198/36204

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 6, 2023

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

It involves the following packages:

  • L1Trigger/L1TMuonEndCap (l1)

@epalencia, @cmsbuild, @aloeliger can you please review it and eventually sign? Thanks.
@dinyar, @missirol, @JanFSchulte, @thomreis, @Martin-Grunewald this is something you requested to watch as well.
@perrotta, @dpiparo, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@aloeliger
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Jul 8, 2023

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-93bef1/33603/summary.html
COMMIT: 3193887
CMSSW: CMSSW_13_2_X_2023-07-07-1100/el8_amd64_gcc11
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/42198/33603/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

There are some workflows for which there are errors in the baseline:
136.897 step 2
140.025 step 2
The results for the comparisons for these workflows could be incomplete
This means most likely that the IB is having errors in the relvals.The error does NOT come from this pull request

Summary:

  • You potentially added 12 lines to the logs
  • Reco comparison results: 7 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3193892
  • DQMHistoTests: Total failures: 6
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3193864
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 207 log files, 159 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

@aloeliger
Copy link
Contributor

+l1

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

@@ -27,6 +27,8 @@ void SectorProcessorLUT::read(bool pc_lut_data, int pc_lut_version) {
coord_lut_dir = "ph_lut_v3_data"; // Update in September 2017 from ReReco alignment, data only
else if (pc_lut_version == 3 && pc_lut_data)
coord_lut_dir = "ph_lut_Run3_2022_data"; // Update in October 2022 from Run 3 2022 alignment, data only
else if (pc_lut_version == 4 && pc_lut_data)
coord_lut_dir = "ph_lut_Run3_2023_data"; // Update in June 2023 from Run 3 2023 alignment, data only
else if (pc_lut_version >= 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't change anything, but it would look healtier as such:

Suggested change
else if (pc_lut_version >= 2)
else if (pc_lut_version >= 5)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This if statement should catcht pc_lut_version >= 2 when pc_lut_data is false. Your suggested version doesn't cover all the cases, if I'm not mistaken. I can clean up the statements next week to simplify them if you prefer that.

Copy link
Contributor

Choose a reason for hiding this comment

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

This if statement should catcht pc_lut_version >= 2 when pc_lut_data is false. Your suggested version doesn't cover all the cases, if I'm not mistaken. I can clean up the statements next week to simplify them if you prefer that.

Correct, I did not pay attention to the condition on pc_lut_data

@@ -27,6 +27,8 @@ void SectorProcessorLUT::read(bool pc_lut_data, int pc_lut_version) {
coord_lut_dir = "ph_lut_v3_data"; // Update in September 2017 from ReReco alignment, data only
else if (pc_lut_version == 3 && pc_lut_data)
coord_lut_dir = "ph_lut_Run3_2022_data"; // Update in October 2022 from Run 3 2022 alignment, data only
else if (pc_lut_version == 4 && pc_lut_data)
coord_lut_dir = "ph_lut_Run3_2023_data"; // Update in June 2023 from Run 3 2023 alignment, data only
Copy link
Contributor

Choose a reason for hiding this comment

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

The PR tests were run without cms-data/L1Trigger-L1TMuon#25, and they did not crash.
Evidently the new version is never tested.
How can we actually test it? Could you at lease please show a log of a workflow run on some data posterior to July 1st, so that we can verify that this does not crash with them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this is a problem with testing these PRs. I'm adding a brief log from re-emulating a recent run. I changed the LogInfo from

edm::LogInfo("L1T") << "EMTF using pc_lut_ver: " << pc_lut_version << ", configured for "
to LogWarning and you can see that in the log as well. Please let me know if you want more validation. Thanks!

11-Jul-2023 08:29:46 CEST  Initiating request to open file root://eoscms.cern.ch//eos/cms/store/data/Run2023D/Muon0/RAW/v1/000/370/332/00000/001535af-e2f9-407a-8e08-16c5b61a2b6b.root
%MSG-w XrdAdaptorInternal:  file_open 11-Jul-2023 08:29:47 CEST pre-events
Failed to open file at URL root://eoscms.cern.ch:1094//eos/cms/store/data/Run2023D/Muon0/RAW/v1/000/370/332/00000/001535af-e2f9-407a-8e08-16c5b61a2b6b.root?xrdcl.requuid=60647035-ea8c-4468-9f6e-06611d8297a4.
%MSG
%MSG-w XrdAdaptorInternal:  file_open 11-Jul-2023 08:29:47 CEST pre-events
Failed to open file at URL root://eoscms.cern.ch:1094//eos/cms/store/data/Run2023D/Muon0/RAW/v1/000/370/332/00000/001535af-e2f9-407a-8e08-16c5b61a2b6b.root?tried=&xrdcl.requuid=10be2b5d-6604-4948-8898-ee157a341e1b.
%MSG
11-Jul-2023 08:29:47 CEST  Initiating request to open file root://xrootd-cms.infn.it//store/data/Run2023D/Muon0/RAW/v1/000/370/332/00000/001535af-e2f9-407a-8e08-16c5b61a2b6b.root
11-Jul-2023 08:29:49 CEST  Successfully opened file root://xrootd-cms.infn.it//store/data/Run2023D/Muon0/RAW/v1/000/370/332/00000/001535af-e2f9-407a-8e08-16c5b61a2b6b.root
Begin processing the 1st record. Run 370332, Event 593319087, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:54.150 CEST
%MSG-w L1T:  L1TMuonEndCapTrackProducer:simEmtfDigisData  11-Jul-2023 08:29:56 CEST Run: 370332 Event: 593319087
EMTF using pc_lut_ver: 4, configured for data
%MSG
2023-07-11 08:29:56.664644: I tensorflow/core/platform/cpu_feature_guard.cc:142] This TensorFlow binary is optimized with oneAPI Deep Neural Network Library (oneDNN) to use the following CPU instructions in performance-critical operations:  SSE4.1 SSE4.2 AVX AVX2 FMA
To enable them in other operations, rebuild TensorFlow with the appropriate compiler flags.
2023-07-11 08:29:56.665110: E tensorflow/stream_executor/cuda/cuda_driver.cc:271] failed call to cuInit: UNKNOWN ERROR (34)
2023-07-11 08:29:56.665150: I tensorflow/stream_executor/cuda/cuda_diagnostics.cc:156] kernel driver does not appear to be running on this host (lxplus724.cern.ch): /proc/driver/nvidia/version does not exist
Begin processing the 2nd record. Run 370332, Event 593411623, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:56.838 CEST
Begin processing the 3rd record. Run 370332, Event 593669958, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:56.919 CEST
Begin processing the 4th record. Run 370332, Event 593989761, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:56.988 CEST
Begin processing the 5th record. Run 370332, Event 593409302, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:57.050 CEST
Begin processing the 6th record. Run 370332, Event 593839778, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:57.107 CEST
Begin processing the 7th record. Run 370332, Event 594204673, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:57.190 CEST
Begin processing the 8th record. Run 370332, Event 593438427, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:57.267 CEST
Begin processing the 9th record. Run 370332, Event 593796134, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:57.346 CEST
Begin processing the 10th record. Run 370332, Event 593326056, LumiSection 260 on stream 0 at 11-Jul-2023 08:29:57.410 CEST
11-Jul-2023 08:29:57 CEST  Closed file root://xrootd-cms.infn.it//store/data/Run2023D/Muon0/RAW/v1/000/370/332/00000/001535af-e2f9-407a-8e08-16c5b61a2b6b.root

@perrotta
Copy link
Contributor

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

4 participants