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

Validation code update for MTD tracks and hits #33252

Merged
merged 8 commits into from
Mar 29, 2021

Conversation

gsorrentino18
Copy link
Contributor

PR description:

This PR expands the validation code for tracks, including monitoring for the following quantities:
-Time and time error of tracks at the different steps of the MTD track reconstruction
-MVA and pathlenght of MTD tracks
-pT resolution wrt General tracks

The validation code for ETL and BTL Reco Hits has also been expanded, adding time and energy resolution MEs and implementing an optional set of histograms for the monitoring of the hits local position, that can be used for dedicated studies.

PR validation:

The new code has been tested in the release CMSSW_11_3_X_2021-03-19-1100

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33252/21720

  • This PR adds an extra 36KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @gsorrentino18 (Giulia Sorrentino) for master.

It involves the following packages:

Validation/Configuration
Validation/MtdValidation

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @mdhildreth, @civanch, @rvenditti can you please review it and eventually sign? Thanks.
@apsallid, @rovere, @fabiocos, @lecriste 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

@civanch
Copy link
Contributor

civanch commented Mar 23, 2021

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d4883c/13713/summary.html
COMMIT: f5c759d
CMSSW: CMSSW_11_3_X_2021-03-23-1400/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33252/13713/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: 14 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2639783
  • DQMHistoTests: Total failures: 30
  • DQMHistoTests: Total nulls: 5
  • DQMHistoTests: Total successes: 2639726
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 21.304 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 23234.0,... ): 16.830 KiB MTD/Tracks
  • DQMHistoSizes: changed ( 23234.0,... ): -13.637 KiB MTD/GlobalReco
  • DQMHistoSizes: changed ( 23234.0,... ): 2.268 KiB MTD/ETL
  • DQMHistoSizes: changed ( 23234.0,... ): -0.136 KiB MTD/BTL
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@civanch
Copy link
Contributor

civanch commented Mar 24, 2021

+1

meHitLongPos_->Fill(recHit.position());
meHitLongPosErr_->Fill(recHit.positionError());

meOccupancy_->Fill(global_point.z(), global_point.phi());

if (LocalPosDebug_) {
meLocalOccupancy_->Fill(local_point.x() + recHit.position(), local_point.y());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gsorrentino18 is the addition of the reconstructed longitudinal position giving the expected behaviour?

Copy link
Contributor Author

@gsorrentino18 gsorrentino18 Mar 24, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following occupancy plot (10k MinBias) shows that the BTL crystals are correctly arranged in the module, but there's still something that needs to be understood in the reconstruction of the longitudinal position inside the crystal.

Schermata 2021-03-24 alle 09 38 51

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is observed likely accounts for uncertainty in the reconstructed position that is used on top of the nominal center of the crystal according to the geometry, this is seen also in the one-dimensional plot of the longitudinal position in local coordinates

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in any case the 16x3 layout is confirmed to be in the correct orientation, unlike the ETL pixels layout (@parbol FYI)

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33252/21784

  • This PR adds an extra 40KB to repository

@cmsbuild
Copy link
Contributor

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

@fabiocos
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d4883c/13800/summary.html
COMMIT: 65147b9
CMSSW: CMSSW_11_3_X_2021-03-26-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33252/13800/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: 11 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2639783
  • DQMHistoTests: Total failures: 16
  • DQMHistoTests: Total nulls: 5
  • DQMHistoTests: Total successes: 2639740
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 36.224 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 23234.0,... ): 17.445 KiB MTD/Tracks
  • DQMHistoSizes: changed ( 23234.0,... ): -13.637 KiB MTD/GlobalReco
  • DQMHistoSizes: changed ( 23234.0,... ): 4.014 KiB MTD/ETL
  • DQMHistoSizes: changed ( 23234.0,... ): 1.232 KiB MTD/BTL
  • DQMHistoSizes: changed ( 312.0 ): 0.004 KiB MessageLogger/Warnings
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@srimanob
Copy link
Contributor

@cms-sw/dqm-l2 @cms-sw/simulation-l2
Do you have comments on this PR? We would like to merge this first, then #33226

@jfernan2
Copy link
Contributor

+1

@civanch
Copy link
Contributor

civanch commented Mar 28, 2021

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

@qliphy
Copy link
Contributor

qliphy commented Mar 29, 2021

+1

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.

7 participants