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

Initialize cut parser earlier in NanoAODDQM module #44573

Merged
merged 2 commits into from
Mar 31, 2024

Conversation

Dr15Jones
Copy link
Contributor

PR description:

IB RelVals have been failing because the cut parser in NanoAODDQM sometimes fails to find methods of nanoaod::FlatTable. This has been seen before and is believe to be a threading issue in ROOT related to asking for the list of methods while other ROOT calls are happening. Moving the initialization to the constructor of the module should avoid this threading problem.

The code was changed so that instead of needing the FlatTable in order to ask it what are the available columns, now the code assumes any name not related to either a function call or a known method of FlatTable::RowView is a column name. This allows the cut parser to be fully initialized in the module constructor.

Additionally, using an invalid column name in a call to FlatTable::RowView::getAnyValue now throws an exception which contains the name of the invalid column.

PR validation:

New unit test passes.

There is a known threading problem involving ROOT where the cut
parser initialization can fail if done during the Event loop.
If an unknown column name is used, issue an exception which
contains the name of the column.
@Dr15Jones
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Mar 28, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-44573/39728

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

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

It involves the following packages:

  • DataFormats/NanoAOD (xpog)
  • PhysicsTools/NanoAOD (xpog)

@vlimant, @hqucms can you please review it and eventually sign? Thanks.
@AnnikaStein, @missirol, @rovere, @gpetruc this is something you requested to watch as well.
@rappoccio, @sextonkennedy, @antoniovilela you are the release manager for this.

cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4bffac/38499/summary.html
COMMIT: ab99828
CMSSW: CMSSW_14_1_X_2024-03-28-1100/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44573/38499/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

  • You potentially added 83 lines to the logs
  • Reco comparison results: 48 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3297437
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3297414
  • 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

@vlimant
Copy link
Contributor

vlimant commented Mar 29, 2024

enable nano

@vlimant
Copy link
Contributor

vlimant commented Mar 29, 2024

please test

@vlimant
Copy link
Contributor

vlimant commented Mar 29, 2024

+1

@cmsbuild
Copy link
Contributor

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. 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)

@cmsbuild
Copy link
Contributor

-1

Failed Tests: RelVals-NANO
Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4bffac/38503/summary.html
COMMIT: ab99828
CMSSW: CMSSW_14_1_X_2024-03-28-2300/el8_amd64_gcc12
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44573/38503/install.sh to create a dev area with all the needed externals and cmssw changes.

RelVals-NANO

  • 2500.02500.0_NANOmc106Xul16v2/step2_NANOmc106Xul16v2.log

Comparison Summary

Summary:

  • You potentially removed 4 lines from the logs
  • Reco comparison results: 45 differences found in the comparisons
  • DQMHistoTests: Total files compared: 48
  • DQMHistoTests: Total histograms compared: 3297469
  • DQMHistoTests: Total failures: 3
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 3297446
  • 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

@vlimant
Copy link
Contributor

vlimant commented Mar 29, 2024

please test
unrelated spurious error

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-4bffac/38508/summary.html
COMMIT: ab99828
CMSSW: CMSSW_14_1_X_2024-03-28-2300/el8_amd64_gcc12
Additional Tests: NANO
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/44573/38508/install.sh to create a dev area with all the needed externals and cmssw changes.

Comparison Summary

Summary:

NANO Comparison Summary

Summary:

  • No significant changes to the logs found
  • 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 51 log files, 29 edm output root files, 15 DQM output files

Nano size comparison Summary:

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.59 4.58 +0.3% 2.201 2.191
2500.001 2.703 2.703 0.000 ( +0.0% ) 4.14 4.06 +2.0% 2.622 2.623
2500.002 2.643 2.643 0.000 ( +0.0% ) 4.28 4.26 +0.6% 2.619 2.606
2500.01 1.318 1.318 0.000 ( +0.0% ) 8.33 8.09 +3.0% 2.316 2.299
2500.011 1.739 1.739 0.000 ( +0.0% ) 4.55 4.54 +0.2% 2.443 2.442
2500.012 1.580 1.580 0.000 ( +0.0% ) 6.48 6.32 +2.5% 2.403 2.366
2500.1 2.196 2.196 0.000 ( +0.0% ) 4.63 4.60 +0.8% 2.096 2.022
2500.2 2.313 2.313 0.000 ( +0.0% ) 5.28 5.23 +0.9% 2.006 1.928
2500.21 1.181 1.181 0.000 ( +0.0% ) 3.53 3.50 +0.7% 2.307 2.238
2500.211 1.545 1.545 0.000 ( +0.0% ) 3.17 3.12 +1.7% 2.286 2.269
2500.3 2.069 2.069 0.000 ( +0.0% ) 10.25 9.89 +3.6% 1.995 1.995
2500.301 2.642 2.642 0.000 ( +0.0% ) 8.72 8.48 +2.8% 1.979 1.982
2500.31 7.159 7.159 0.000 ( +0.0% ) 1.39 1.40 -0.3% 1.730 1.730
2500.311 1.564 1.564 0.000 ( +0.0% ) 6.69 7.26 -7.8% 1.082 1.083
2500.312 540.453 540.453 0.000 ( +0.0% ) 0.52 0.51 +2.5% 1.625 1.621
2500.313 817.689 817.689 0.000 ( +0.0% ) 0.73 0.70 +4.3% 1.618 1.611
2500.32 1.255 1.255 0.000 ( +0.0% ) 13.05 12.48 +4.5% 2.352 2.347
2500.321 1.642 1.642 0.000 ( +0.0% ) 9.70 9.59 +1.2% 2.403 2.432
2500.322 1.163 1.163 0.000 ( +0.0% ) 10.33 9.98 +3.4% 2.258 2.220
2500.323 1.758 1.758 0.000 ( +0.0% ) 10.05 9.74 +3.2% 2.157 2.204
2500.324 3.093 3.093 0.000 ( +0.0% ) 1.81 1.75 +3.2% 2.120 2.126
2500.325 1.789 1.789 0.000 ( +0.0% ) 10.29 9.79 +5.0% 2.336 2.323
2500.4 2.227 2.227 0.000 ( +0.0% ) 9.85 9.53 +3.4% 1.874 1.863
2500.401 1.755 1.755 0.000 ( +0.0% ) 8.58 7.99 +7.4% 1.737 1.719
2500.402 2.771 2.771 0.000 ( +0.0% ) 8.29 7.90 +4.9% 1.868 1.878
2500.403 5.081 5.081 0.000 ( +0.0% ) 1.34 1.32 +1.8% 1.875 1.873
2500.404 2.779 2.779 0.000 ( +0.0% ) 8.31 7.76 +7.1% 1.771 1.766
2500.5 4.936 4.936 0.000 ( +0.0% ) 15.82 15.47 +2.3% 1.331 1.335
2500.51 8.960 8.960 0.000 ( +0.0% ) 9.66 9.58 +0.9% 1.424 1.361

@antoniovilela
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 968fe1c into cms-sw:master Mar 31, 2024
13 checks passed
@Dr15Jones Dr15Jones deleted the earlyInitNanoAODDQM branch April 16, 2024 15:02
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

4 participants