cmssw icon indicating copy to clipboard operation
cmssw copied to clipboard

Hlt gen validation

Open finnlabe opened this issue 3 years ago • 11 comments

PR description:

This PR adds a validation module that produces trigger efficiency histograms as function of generator-level particle properties. It will be used to get a generic, very quick and easy look at the HLT output to check if any new release or change to the HLT introduced issues. Information on the module is gathered in this Twiki page.

Two modules are included, the HLTGenValSource which produces histograms before and after trigger filter application, and a HLTGenValClient module, creating efficiency histograms from the sources outputs.

The module is standalone, should not change any other unrelated modules and does not rely on external code or other PRs. Development has happened within the STEAM subgroup of the TSG, overseen by @Sam-Harper and Lisa Benato. Aside from the other required review steps, the module is still awaiting review by the TSG experts, including Sam.

PR validation:

The code and its outputs have been tested offline. Additionally, the steps described in the Workflow for pull requests to CMSSW document were performed and the code should hopefully be readable and fulfill the CMS coding requirements - if not I'd be happy to make any required changes.

The PR is targeting to be included in the next possible release.

finnlabe avatar Jul 04 '22 14:07 finnlabe

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/30849

  • This PR adds an extra 32KB to repository

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

  • code-format: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/30849/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/30849/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Jul 04 '22 14:07 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/30852

  • This PR adds an extra 36KB to repository

cmsbuild avatar Jul 05 '22 07:07 cmsbuild

A new Pull Request was created by @finnlabe (Finn Labe) for master.

It involves the following packages:

  • Validation/HLTrigger (****)

The following packages do not have a category, yet:

Validation/HLTrigger 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. @perrotta, @dpiparo, @qliphy you are the release manager for this.

cms-bot commands are listed here

cmsbuild avatar Jul 05 '22 07:07 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31120

  • This PR adds an extra 28KB to repository

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

  • code-format: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31120/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31120/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Jul 18 '22 20:07 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31121

  • This PR adds an extra 32KB to repository

cmsbuild avatar Jul 18 '22 21:07 cmsbuild

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

cmsbuild avatar Jul 18 '22 21:07 cmsbuild

assign dqm

emanueleusai avatar Aug 02 '22 06:08 emanueleusai

New categories assigned: dqm

@jfernan2,@ahmad3213,@micsucmed,@rvenditti,@emanueleusai,@pmandrik you have been requested to review this Pull request/Issue and eventually sign? Thanks

cmsbuild avatar Aug 02 '22 06:08 cmsbuild

please test

emanueleusai avatar Aug 02 '22 06:08 emanueleusai

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bb954b/26586/summary.html COMMIT: 4b9508dcd6ad1f80e409784bca6364c307e99941 CMSSW: CMSSW_12_5_X_2022-08-01-2300/el8_amd64_gcc10 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38593/26586/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: 1042.0 step 3 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:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3669004
  • DQMHistoTests: Total failures: 2
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3668980
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 210 log files, 47 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

cmsbuild avatar Aug 02 '22 10:08 cmsbuild

+dqm

emanueleusai avatar Aug 02 '22 21:08 emanueleusai

Hi @makortel, thank you for reviewing this pull request! I have changed the way of getting the CMSSW version.

On the topic of keeping the code in interface or moving to plugins, after discussing with @Sam-Harper: All classes that are in the interface directory are general enough that they could be used in other packages. Therefore, to keep them separated from the actual plugins, we'd prefer to keep them there. However there are no current plans from our side for outside usage. Therefore, if you'd prefer, it would not be an issue to move them to plugins.

finnlabe avatar Aug 18 '22 07:08 finnlabe

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31631

  • This PR adds an extra 48KB to repository

cmsbuild avatar Aug 18 '22 07:08 cmsbuild

Pull request #38593 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please check and sign again.

cmsbuild avatar Aug 18 '22 07:08 cmsbuild

please test

emanueleusai avatar Aug 22 '22 20:08 emanueleusai

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bb954b/26996/summary.html COMMIT: 5ab4414f403fe6d9d81f86f666d3cfae4ab34c32 CMSSW: CMSSW_12_5_X_2022-08-22-1100/el8_amd64_gcc10 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38593/26996/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 6 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3693040
  • DQMHistoTests: Total failures: 13
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3693004
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -0.004 KiB( 50 files compared)
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

cmsbuild avatar Aug 23 '22 00:08 cmsbuild

+1

emanueleusai avatar Aug 23 '22 19:08 emanueleusai

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

cmsbuild avatar Aug 23 '22 19:08 cmsbuild

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31789

  • This PR adds an extra 20KB to repository

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

  • code-format: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31789/code-format.patch e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31789/code-format.patch | patch -p1 You can also run scram build code-format to apply code format directly

cmsbuild avatar Aug 25 '22 08:08 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31790

  • This PR adds an extra 24KB to repository

cmsbuild avatar Aug 25 '22 08:08 cmsbuild

Pull request #38593 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please check and sign again.

cmsbuild avatar Aug 25 '22 08:08 cmsbuild

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38593/31799

  • This PR adds an extra 88KB to repository

cmsbuild avatar Aug 25 '22 12:08 cmsbuild

Pull request #38593 was updated. @emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @micsucmed, @rvenditti can you please check and sign again.

cmsbuild avatar Aug 25 '22 12:08 cmsbuild

Thanks for the additional comments! I made the cleanup changes, and will add a separate commit in a second switching to untracked parameters.

finnlabe avatar Aug 25 '22 12:08 finnlabe

Thanks for the additional comments! I made the cleanup changes, and will add a separate commit in a second switching to untracked parameters.

Waiting for that commit before launching the tests...

perrotta avatar Aug 25 '22 14:08 perrotta

Thanks and apologies, making that change took a bit longer than expected. The reason is that the new code relies on the VarRangeCut class, which itself implements tracked parameters (see here).

I discussed quickly with @Sam-Harper, who wrote that class and the modules using it that it should be possible to make these untracked as well. However, this would require a very large number of changes to various classes using this, and config files specifying rangeCuts... I started doing that, but it takes more time that I have available today (and is error-prone as its hard to catch every place where its used).

For that reason, I would prefer keeping everything tracked for the moment, if you'd prefer however I could make everything but the VarRangeCuts untracked.

finnlabe avatar Aug 25 '22 15:08 finnlabe

please test (let postpone the migration to a possible future PR)

perrotta avatar Aug 25 '22 16:08 perrotta

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-bb954b/27095/summary.html COMMIT: f58997d1e82ce3fe7217997f710906b4000a62c3 CMSSW: CMSSW_12_5_X_2022-08-25-1100/el8_amd64_gcc10 User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/38593/27095/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • No significant changes to the logs found
  • Reco comparison results: 4 differences found in the comparisons
  • DQMHistoTests: Total files compared: 51
  • DQMHistoTests: Total histograms compared: 3693084
  • DQMHistoTests: Total failures: 8
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3693054
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 0.0 KiB( 50 files compared)
  • Checked 212 log files, 49 edm output root files, 51 DQM output files
  • TriggerResults: no differences found

cmsbuild avatar Aug 25 '22 20:08 cmsbuild

+1

emanueleusai avatar Aug 29 '22 18:08 emanueleusai

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)

cmsbuild avatar Aug 29 '22 18:08 cmsbuild