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

MTV fast cleaning #33500

Merged
merged 3 commits into from May 11, 2021
Merged

MTV fast cleaning #33500

merged 3 commits into from May 11, 2021

Conversation

mtosi
Copy link
Contributor

@mtosi mtosi commented Apr 22, 2021

PR description:

the Multi Track Validator (MTV) is one of the main computational and memory offenders in the VALIDATION step.
there are some not that necessary plots, and with this PR they are removed
and a new flag has been added in order to make the dzcut related performance plots on-demand

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33500/22234

  • This PR adds an extra 40KB to repository

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-33500/22239

  • This PR adds an extra 36KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @mtosi (mia tosi) for master.

It involves the following packages:

Validation/RecoTrack

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks.
@makortel, @felicepantaleo, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @wmtford, @ebrondol, @dgulhan 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

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13f6e8/14426/summary.html
COMMIT: 723a161
CMSSW: CMSSW_12_0_X_2021-04-22-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33500/14426/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: 8 differences found in the comparisons
  • DQMHistoTests: Total files compared: 38
  • DQMHistoTests: Total histograms compared: 2861106
  • DQMHistoTests: Total failures: 7944
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2853139
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -9231.178 KiB( 37 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -345.449 KiB Tracking/Track
  • DQMHistoSizes: changed ( 10024.0,... ): -164.155 KiB Tracking/TrackTPPtLess09
  • DQMHistoSizes: changed ( 1306.0,... ): -276.117 KiB Tracking/Track
  • DQMHistoSizes: changed ( 1306.0,... ): -128.921 KiB Tracking/TrackTPPtLess09
  • DQMHistoSizes: changed ( 23234.0,... ): -229.883 KiB Tracking/Track
  • DQMHistoSizes: changed ( 23234.0,... ): -117.061 KiB Tracking/TrackTPEtaGreater2p7
  • DQMHistoSizes: changed ( 23234.0,... ): -105.425 KiB Tracking/TrackTPPtLess09
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 160 log files, 37 edm output root files, 38 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

Thanks for this PR @mtosi !!
Apart from the plot removal, I see that some histograms are also being modified, mainly efficiencies, I understand they are expected due to the changes introduced, can you confirm?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_0_X_2021-04-22-1100+13f6e8/42364/dqm-histo-comparison-summary.html

@jfernan2
Copy link
Contributor

ping @mtosi

@silviodonato
Copy link
Contributor

kind reminder @mtosi

@mtosi
Copy link
Contributor Author

mtosi commented May 4, 2021

apologies for the delay, there was a "feature" (aka bug), indeed
thanks for the support and patience

it should be fixed, now

@cmsbuild
Copy link
Contributor

cmsbuild commented May 4, 2021

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33500/22472

  • This PR adds an extra 36KB to repository

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

@cmsbuild
Copy link
Contributor

cmsbuild commented May 4, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33500/22474

  • This PR adds an extra 28KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented May 4, 2021

Pull request #33500 was updated. @andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please check and sign again.

@qliphy
Copy link
Contributor

qliphy commented May 9, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented May 9, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-13f6e8/14950/summary.html
COMMIT: d71cba3
CMSSW: CMSSW_12_0_X_2021-05-09-0000/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33500/14950/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: 2 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2648114
  • DQMHistoTests: Total failures: 1963
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2646129
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: -8721.57 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): -345.449 KiB Tracking/Track
  • DQMHistoSizes: changed ( 10024.0,... ): -164.155 KiB Tracking/TrackTPPtLess09
  • DQMHistoSizes: changed ( 1306.0,... ): -276.117 KiB Tracking/Track
  • DQMHistoSizes: changed ( 1306.0,... ): -128.921 KiB Tracking/TrackTPPtLess09
  • DQMHistoSizes: changed ( 23234.0,... ): -229.883 KiB Tracking/Track
  • DQMHistoSizes: changed ( 23234.0,... ): -117.061 KiB Tracking/TrackTPEtaGreater2p7
  • DQMHistoSizes: changed ( 23234.0,... ): -105.425 KiB Tracking/TrackTPPtLess09
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@jfernan2
Copy link
Contributor

+1
Thanks a lot @mtosi

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

@silviodonato
Copy link
Contributor

I see that this PR, for example in 10224.0_TTbar_13+2017PU+TTbar_13TeV_TuneCUETP8M1_GenSim+DigiPU+RecoFakeHLTPU+HARVESTFakeHLTPU+Nano:

  • Remove 660 plots in Tracks.
    Specifically, the following 11 plots in 60 subfolders
effic_vs_dzpvsigcut
effic_vs_dzpvsigcut2
fakerate_vs_dzpvsigcut
globalEfficiencies
num_assoc(recoToSim)_dzpvsigcut
num_assoc(simToReco)_dzpvsigcut
num_pileup_dzpvsigcut
num_reco_dzpvsigcut
num_simul2_dzpvsigcut
num_simul_dzpvsigcut
pileuprate_dzpvsigcut
  • Remove 336 plots in TrackTPPtLess09.
    Specifically, the following 6 plots in 56 subfolders
effic_vs_dzpvsigcut
effic_vs_dzpvsigcut2
globalEfficiencies
num_assoc(simToReco)_dzpvsigcut
num_simul2_dzpvsigcut
num_simul_dzpvsigcut
  • Change 60 globalEfficiencies plots in Tracks and 56 globalEfficiencies plots TrackTPPtLess09 (see example below)

Many of the removed plots were not empty.

@mtosi could you confirm that these changes are expected? Thanks

image

@VinInn
Copy link
Contributor

VinInn commented May 11, 2021

I read "a new flag has been added in order to make the dzcut related performance plots on-demand" and understand they are now off by default.

@qliphy
Copy link
Contributor

qliphy commented May 11, 2021

+1

@cmsbuild cmsbuild merged commit f0cac19 into cms-sw:master May 11, 2021
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

6 participants