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 Validation: expand track-hits matching efficiency #38444

Merged
merged 12 commits into from Jun 22, 2022

Conversation

fabiocos
Copy link
Contributor

PR description:

This PR proposes the addition of monitoring histograms devoted to the study of the track - MTD hits matching efficiency. In particular, it adds a check for tracks in ETL with hits in both forward discs, and an alternative study using reconstructed tracks, matched with a TrackingParticle which is found to have given origin to MTD hits. This association is built in Validation/MtdTracksValidation based on the track and event id stored in PSimHit. A dedicated study, switched off by default and activated with a flag optionalPlots allows the study of the performance of this matching approach.

A BTL local reconstruction harvesting is also added to study the BTL reconstructed hits occupancy as a function of energy.

PR validation:

The code has been extensively used on 100k single muons and pions (private production) and on RelVal TTbar events from CMSSW_12_5_0_pre2.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-38444/30645

  • This PR adds an extra 48KB to repository

@cmsbuild
Copy link
Contributor

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, @micsucmed, @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

please test

@fabiocos
Copy link
Contributor Author

@gsorrentino18 FYI

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-cc33a0/25647/summary.html
COMMIT: a5b4490
CMSSW: CMSSW_12_5_X_2022-06-20-1100/el8_amd64_gcc10
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/38444/25647/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: 50
  • DQMHistoTests: Total histograms compared: 3659099
  • DQMHistoTests: Total failures: 109
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 3658967
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 61.01200000000001 KiB( 49 files compared)
  • DQMHistoSizes: changed ( 23234.0,... ): 6.885 KiB MTD/Tracks
  • DQMHistoSizes: changed ( 23234.0,... ): 0.742 KiB MTD/BTL
  • DQMHistoSizes: changed ( 312.0 ): -0.004 KiB MessageLogger/Warnings
  • Checked 208 log files, 45 edm output root files, 50 DQM output files
  • TriggerResults: no differences found

@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)

@emanueleusai
Copy link
Member

@fabiocos are you planning a 12_4 backport?

@perrotta
Copy link
Contributor

  • It looks like this PR does its job, e.g. from wf 35034.0:
    image
  • @fabiocos out of curiosity, do you understand why there are a couple of entries less in the bin at eta=0?

@fabiocos
Copy link
Contributor Author

@emanueleusai a backport makes sense only if this is needed by some production monitoring. @cms-sw/upgrade-l2 do we have any particularly relevant at the horizon with 12_4_0, besides the regular tests?

@fabiocos
Copy link
Contributor Author

@perrotta there are several new ingredients into this PR, indeed fixing the missing ETL for TP with matched MTD hits was part of this (a trivial mismatch of collections on which to loop at https://github.com/cms-sw/cmssw/blob/master/Validation/MtdValidation/plugins/MtdTracksValidation.cc#L406 ).

Concerning to the non perfect identity of the barrel part, one possible cause could be the additional selection we do for TP, excluding those corresponding to SimTracks outside the tracker volume, see method mvaTPSel. Also the logic of the identification of the origin track of a hit has been improved, but since the test you show does not contain pileup, adding the eventId should be irrelevant.

@fabiocos
Copy link
Contributor Author

In general as you see only MtdTrackValidation is affected by changes, as expected. I have tried to keep as much as possible backward compatibility in the efficiency definition, and added only additional plots for instance for forward association with exactly two hits ( @gsorrentino18 please take note in view of future release validations )

@perrotta
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit c3498e8 into cms-sw:master Jun 22, 2022
@fabiocos fabiocos deleted the fc-fixvalid-dev branch June 23, 2022 07:30
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