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

MTD reconstruction validation: tracks and vertices #35989

Merged
merged 14 commits into from Nov 11, 2021

Conversation

fabiocos
Copy link
Contributor

@fabiocos fabiocos commented Nov 4, 2021

PR description:

This PR extends the previous work integrated in #35482 in several ways:

  • cleaning of existing plots for 4D vertex validation;
  • move into vertex class plots for vertex validation previously in track validation;
  • addition in track validation class of plots based on a reco-sim truth matching using GenParticles associated to the true PV, as done for the selection sued to compute the MVA MTD quality flag.

An harvester has been introduced also for MtdTracksValidation class

PR validation:

Test wf 34634.0 and 34834.0 (PU privately built) runs and produces expected plots.

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 4, 2021

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-35989/26419

  • This PR adds an extra 48KB to repository

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 4, 2021

A new Pull Request was created by @fabiocos (Fabio Cossutti) for master.

It involves the following packages:

  • Validation/MtdValidation (dqm)

@emanueleusai, @ahmad3213, @cmsbuild, @jfernan2, @pmandrik, @pbo0, @rvenditti 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

@fabiocos
Copy link
Contributor Author

fabiocos commented Nov 4, 2021

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Nov 4, 2021

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-2ba253/20258/summary.html
COMMIT: 22b242c
CMSSW: CMSSW_12_2_X_2021-11-04-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/35989/20258/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: 42
  • DQMHistoTests: Total histograms compared: 2901845
  • DQMHistoTests: Total failures: 15
  • DQMHistoTests: Total nulls: 33
  • DQMHistoTests: Total successes: 2901775
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 43.731 KiB( 41 files compared)
  • DQMHistoSizes: changed ( 23234.0,... ): 5.390 KiB MTD/Vertices
  • DQMHistoSizes: changed ( 23234.0,... ): 3.357 KiB MTD/Tracks
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 177 log files, 37 edm output root files, 42 DQM output files
  • TriggerResults: no differences found

@fabiocos
Copy link
Contributor Author

fabiocos commented Nov 5, 2021

@gsorrentino18 FYI

@fabiocos
Copy link
Contributor Author

fabiocos commented Nov 9, 2021

Any comment? Otherwise can we move forward?

@emanueleusai
Copy link
Member

@fabiocos can you please greenlight the new MEs here?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_12_2_X_2021-11-04-1100+2ba253/46670/dqm-histo-comparison-summary.html
especially:
https://tinyurl.com/yzesl6lu and https://tinyurl.com/ykxfjbw9

Some of the new ME appear to be empty, is this because of the small statistics of the unit tests?

@fabiocos
Copy link
Contributor Author

@emanueleusai I did extensive checks of the histograms, and they look reasonable to me. Besides the newly added histograms, those removed or whose definition has been changed, all the tests involving multiple reconstructed vertices are empty if there is just a single reconstructed vertex (for no PU samples).

For samples with PU, some histograms (DeltaZ and DeltaT between vertices) may anyway have a limited number of entries for low PU. In particular DeltaT histograms are filled only for vertices close in Z, see for instance https://github.com/cms-sw/cmssw/pull/35989/files#diff-ad794caa0fa59947d715316dd1617c612e41544f6f43043f13b5a28321e81d1fR1406 .This could explain why even for PU samples some of these histograms are not filled. The idea is to monitor to which extent the time information may separate vertices very close in Z.

To give you an idea, in a private test with PU = 200, for 5 events I find just 1 entry for DeltaTfakefake, and 71 for DeltaTfakereal.

@fabiocos
Copy link
Contributor Author

DeltaTfakefake.pdf

100 events with PU = 200 (comparing this PR and an additional update to the time reconstruction to be proposed soon)

@emanueleusai
Copy link
Member

Thank you very much for the feedback!

@emanueleusai
Copy link
Member

+1

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

@qliphy
Copy link
Contributor

qliphy commented Nov 11, 2021

+1

@cmsbuild cmsbuild merged commit 31162c9 into cms-sw:master Nov 11, 2021
@fabiocos fabiocos deleted the fc-4dvtxvalid branch November 11, 2021 08:56
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