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

Customized L1T NanoAOD #43957

Merged
merged 12 commits into from
Apr 8, 2024
Merged

Customized L1T NanoAOD #43957

merged 12 commits into from
Apr 8, 2024

Conversation

lathomas
Copy link
Contributor

PR description:

This package allows to save Level 1 Trigger information (unpacked or reemulated) in a standard NANOAOD format. A few examples of cmsDriver commands are provided for Run 3 data and simulated samples in the README file.

The current PR focuses on Phase 1 L1 objects (unpacked or reemulated), and calorimeter trigger primitives and trigger towers. It is expected to be completed in the future with additional information (e.g. on muon TPs).

PR validation:

The cmsDriver.py commands in the README are found to properly run.

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 13, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43957/38843

  • This PR adds an extra 20KB to repository

  • Found files with invalid states:

    • DPGAnalysis/L1TNanoAOD/python/customL1nano.pybu:
    • DPGAnalysis/L1TNanoAOD/plugins/ZGammaJetInfoProducer.cc:

Code check has found code style and quality issues which could be resolved by applying following patch(s)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43957/38844

  • This PR adds an extra 20KB to repository

  • Found files with invalid states:

    • DPGAnalysis/L1TNanoAOD/python/customL1nano.pybu:
    • DPGAnalysis/L1TNanoAOD/plugins/ZGammaJetInfoProducer.cc:

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • DPGAnalysis/L1TNanoAOD (****)

The following packages do not have a category, yet:

DPGAnalysis/L1TNanoAOD
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.
@missirol this is something you requested to watch as well.
@sextonkennedy, @antoniovilela, @rappoccio you are the release manager for this.

cms-bot commands are listed here

@lathomas
Copy link
Contributor Author

I guess this should be assigned to XPOG ? @vlimant @hqucms
FYI @eyigitba @slaurila @aloeliger @epalencia @artlbv

@vlimant
Copy link
Contributor

vlimant commented Feb 14, 2024

assign xpog
indeed

@cmsbuild
Copy link
Contributor

New categories assigned: xpog

@vlimant,@hqucms you have been requested to review this Pull request/Issue and eventually sign? Thanks

@vlimant
Copy link
Contributor

vlimant commented Feb 14, 2024

The first two recommendations are to follow the NANO:@L1T and to add a test workflow in the nano matrix. Can you please explain the difference with respect to #40663 ?

@lathomas
Copy link
Contributor Author

#40663 allows to add the unpacked L1 objects to central NANO, starting from MINIAOD. This PR is much more generic: it allows to define a custom NANO format, that stores both emulated and unpacked L1 objects, but also the Trigger Primitives (calo only for now). This package aims at replacing https://github.com/cms-sw/cmssw/tree/master/L1Trigger/L1TNtuples/

@lathomas
Copy link
Contributor Author

About the test workflow, I guess I need to add some lines like HCAL did in https://github.com/cms-sw/cmssw/blob/master/Configuration/PyReleaseValidation/python/relval_production.py#L41-L42
What's the convention for the workflow number?

@lathomas
Copy link
Contributor Author

follow the NANO:@L1T

This I didn't really get, sorry. Could you provide an example of what you mean?

@vlimant
Copy link
Contributor

vlimant commented Feb 14, 2024

please see #43065 for the test and the NANO:@L1T syntax (for lack of better set of instructions)

@artlbv
Copy link
Contributor

artlbv commented Feb 14, 2024 via email

This comment was marked as outdated.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-43957/39484

  • This PR adds an extra 24KB to repository

  • Found files with invalid states:

    • DPGAnalysis/L1TNanoAOD/python/customL1nano.pybu:
    • DPGAnalysis/L1TNanoAOD/plugins/ZGammaJetInfoProducer.cc:

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 2, 2024

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-c45c51/38550/summary.html
COMMIT: 8c7820d
CMSSW: CMSSW_14_1_X_2024-04-02-1100/el8_amd64_gcc12
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/43957/38550/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 7 lines to the logs
  • Reco comparison results: 55 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3297469
  • DQMHistoTests: Total failures: 9
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3297440
  • DQMHistoTests: Total skipped: 20
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 47 files compared)
  • Checked 202 log files, 165 edm output root files, 48 DQM output files
  • TriggerResults: no differences found

NANO Comparison Summary

Summary:

  • You potentially added 5 lines to the logs
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 15
  • DQMHistoTests: Total histograms compared: 16430
  • DQMHistoTests: Total failures: 0
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 16430
  • DQMHistoTests: Total skipped: 0
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 14 files compared)
  • Checked 48 log files, 26 edm output root files, 15 DQM output files

Nano size comparison Summary:

  • Nano ERROR: Missing ref/2500.326-size.json
  • Nano ERROR: Missing ref/2500.326-size.json
    | Sample | kb/ev | ref kb/ev | diff kb/ev | ev/s/thd | ref ev/s/thd | diff rate | mem/thd | ref mem/thd |
    | --- | --- | --- | --- | --- | --- | --- | --- | --- |
    | 2500.0 | 2.553 | 2.553 | 0.000 ( +0.0% ) | 4.63 | 4.65 | -0.5% | 2.172 | 2.225 |
    | 2500.001 | 2.703 | 2.703 | 0.000 ( +0.0% ) | 4.12 | 4.15 | -0.7% | 2.602 | 2.590 |
    | 2500.002 | 2.643 | 2.643 | 0.000 ( +0.0% ) | 4.29 | 4.33 | -0.8% | 2.587 | 2.638 |
    | 2500.01 | 1.318 | 1.318 | 0.000 ( +0.0% ) | 8.13 | 8.38 | -3.0% | 2.272 | 2.292 |
    | 2500.011 | 1.739 | 1.739 | 0.000 ( +0.0% ) | 4.61 | 4.55 | +1.3% | 2.446 | 2.415 |
    | 2500.012 | 1.580 | 1.580 | 0.000 ( +0.0% ) | 6.53 | 6.49 | +0.5% | 2.423 | 2.401 |
    | 2500.1 | 2.196 | 2.196 | 0.000 ( +0.0% ) | 4.62 | 4.65 | -0.7% | 2.072 | 2.070 |
    | 2500.2 | 2.313 | 2.313 | 0.000 ( +0.0% ) | 5.22 | 5.33 | -2.2% | 1.976 | 1.978 |
    | 2500.21 | 1.181 | 1.181 | 0.000 ( +0.0% ) | 3.52 | 3.54 | -0.4% | 2.261 | 2.279 |
    | 2500.211 | 1.545 | 1.545 | 0.000 ( +0.0% ) | 3.16 | 3.18 | -0.4% | 2.357 | 2.357 |
    | 2500.3 | 2.069 | 2.069 | 0.000 ( +0.0% ) | 10.13 | 10.24 | -1.1% | 1.968 | 1.966 |
    | 2500.301 | 2.642 | 2.642 | 0.000 ( +0.0% ) | 8.56 | 8.73 | -1.9% | 1.955 | 1.957 |
    | 2500.31 | 7.159 | 7.159 | 0.000 ( +0.0% ) | 1.35 | 1.42 | -4.9% | 1.707 | 1.702 |
    | 2500.311 | 1.564 | 1.564 | 0.000 ( +0.0% ) | 7.04 | 7.09 | -0.8% | 1.059 | 1.052 |
    | 2500.312 | 540.453 | 540.453 | 0.000 ( +0.0% ) | 0.52 | 0.53 | -2.8% | 1.605 | 1.623 |
    | 2500.313 | 817.689 | 817.689 | 0.000 ( +0.0% ) | 0.72 | 0.73 | -1.9% | 1.588 | 1.575 |
    | 2500.32 | 1.255 | 1.255 | 0.000 ( +0.0% ) | 12.67 | 13.01 | -2.6% | 2.315 | 2.324 |
    | 2500.321 | 1.642 | 1.642 | 0.000 ( +0.0% ) | 9.59 | 9.11 | +5.3% | 2.402 | 2.378 |
    | 2500.322 | 1.163 | 1.163 | 0.000 ( +0.0% ) | 10.11 | 10.46 | -3.4% | 2.133 | 2.167 |
    | 2500.323 | 7.768 | 1.758 | 6.010 ( +341.9% ) | 2.95 | 10.19 | -71.1% | 1.923 | 2.125 |
    | 2500.324 | 1.758 | 3.093 | -1.336 ( -43.2% ) | 9.51 | 1.81 | +425.6% | 2.156 | 2.092 |
    | 2500.325 | 3.093 | 1.789 | 1.304 ( +72.9% ) | 1.76 | 10.41 | -83.1% | 2.088 | 2.331 |
    | 2500.4 | 2.227 | 2.227 | 0.000 ( +0.0% ) | 9.42 | 9.85 | -4.4% | 1.905 | 1.851 |
    | 2500.401 | 1.755 | 1.755 | 0.000 ( +0.0% ) | 8.40 | 8.56 | -1.8% | 1.826 | 1.696 |
    | 2500.402 | 2.771 | 2.771 | 0.000 ( +0.0% ) | 8.15 | 8.32 | -2.1% | 1.869 | 1.843 |
    | 2500.403 | 5.081 | 5.081 | 0.000 ( +0.0% ) | 1.32 | 1.35 | -1.7% | 1.823 | 1.855 |
    | 2500.404 | 2.779 | 2.779 | 0.000 ( +0.0% ) | 7.97 | 8.31 | -4.0% | 1.743 | 1.745 |
    | 2500.5 | 4.936 | 4.936 | 0.000 ( +0.0% ) | 15.79 | 15.94 | -0.9% | 1.441 | 1.305 |
    | 2500.51 | 8.960 | 8.960 | 0.000 ( +0.0% ) | 9.61 | 9.64 | -0.3% | 1.327 | 1.396 |

@aloeliger
Copy link
Contributor

+l1

  • re-sign

@lathomas
Copy link
Contributor Author

lathomas commented Apr 5, 2024

@AdrianoDee @srimanob can you take a look and sign if happy? Thanks !

@srimanob
Copy link
Contributor

srimanob commented Apr 5, 2024

+Upgrade

@AdrianoDee
Copy link
Contributor

Just a remark for the future: unless really needed I would append the new wfs rather than placing them in the middle resulting in a change to the wf numbering. Maybe in this case it is just an artifact due to the multiple concurrent PRs and the need to rebase, but still.

@AdrianoDee
Copy link
Contributor

+pdmv

@cmsbuild
Copy link
Contributor

cmsbuild commented Apr 5, 2024

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

@antoniovilela
Copy link
Contributor

+1

@vlimant
Copy link
Contributor

vlimant commented Apr 9, 2024

I am puzzled how tests succeeded here and lead to #44666 ...

@artlbv
Copy link
Contributor

artlbv commented Apr 9, 2024

This PR was tested with CMSSW_14_1_X_2024-04-02-1100, but the #44666 was merged few hours later (4pm) #44391 (comment) .. bad luck

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.

8 participants