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

[TRK POG Validation] Added residuals vs phi in MTV histos #28971

Merged
merged 2 commits into from Feb 24, 2020

Conversation

emiglior
Copy link
Contributor

PR description:

This PR adds histos of residuals vs phi in MultiTrackValidator useful to monitor tracking performance with evolving phase2 Tracker layouts

No deps on other PRs

PR validation:

Code checks: scram b code-checks
Validated with runTheMatrix

This PR is sot a backport

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28971/13802

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

@cmsbuild
Copy link
Contributor

The code-checks are being triggered in jenkins.

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-28971/13803

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @emiglior (Ernesto Migliore) for master.

It involves the following packages:

Validation/RecoMuon
Validation/RecoTrack

@andrius-k, @kmaeshima, @schneiml, @cmsbuild, @jfernan2, @fioriNTU can you please review it and eventually sign? Thanks.
@Fedespring, @battibass, @makortel, @felicepantaleo, @abbiendi, @GiacomoSguazzoni, @rovere, @VinInn, @calderona, @cericeci, @HuguesBrun, @jhgoh, @wmtford, @mtosi, @ebrondol, @trocino, @dgulhan, @folguera, @rociovilar this is something you requested to watch as well.
@davidlange6, @silviodonato, @fabiocos you are the release manager for this.

cms-bot commands are listed here

@jfernan2
Copy link
Contributor

Hi @emiglior, could you please make sure that subsystem name appears in the title of the PR?

@jfernan2
Copy link
Contributor

Ernesto, we don't have you identified as Tracking DQM Validation contact. Please add yourself to the appropiate e-group:
https://twiki.cern.ch/twiki/bin/viewauth/CMS/DQMContacts#Tracking

@emiglior
Copy link
Contributor Author

emiglior commented Feb 17, 2020

@jfernan2
About changing the title of the PR, is it ok just editing it in the web interface or is there some other prescription to be followed? I guess pre-pending "[TRK POG Validation]"
About e-group membership: is this cms-dqm-offline-developers-tracking the right e-group?
(BTW: very likely, mine will be an occasional contribution...)

@jfernan2
Copy link
Contributor

jfernan2 commented Feb 17, 2020

@jfernan2
About changing the title of the PR, is it ok just editing it in the web interface or is there some other prescription to be followed? I guess pre-pending "[TRK POG Validation]"

Correct. It is just to keep track of where it belongs to. Thanks

About e-group membership: is this cms-dqm-offline-developers-tracking the right e-group?

I believe cms-dqm-validation-developers-tracking matches better since this is Validation

(BTW: very likely, mine will be an occasional contribution...)

OK, but please do it in anycase

@emiglior emiglior changed the title Added residuals vs phi in MTV histos [TRK POG Validation] Added residuals vs phi in MTV histos Feb 17, 2020
@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

cmsbuild commented Feb 18, 2020

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/4723/console Started: 2020/02/18 09:25

@cmsbuild
Copy link
Contributor

+1
Tested at: 200cbeb
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-73c518/4723/summary.html
CMSSW: CMSSW_11_1_X_2020-02-17-2300
SCRAM_ARCH: slc7_amd64_gcc820

@cmsbuild
Copy link
Contributor

Comparison job queued.

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-73c518/4723/summary.html

Comparison Summary:

  • No significant changes to the logs found
  • Reco comparison results: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 34
  • DQMHistoTests: Total histograms compared: 2694086
  • DQMHistoTests: Total failures: 5073
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2688694
  • DQMHistoTests: Total skipped: 319
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 22665.647 KiB( 33 files compared)
  • DQMHistoSizes: changed ( 10024.0,... ): 683.642 KiB HLT/Tracking
  • DQMHistoSizes: changed ( 10024.0,... ): 391.103 KiB HLT/Muon
  • DQMHistoSizes: changed ( 10024.0,... ): 390.709 KiB Tracking/TrackFromPV
  • DQMHistoSizes: changed ( 10024.0,... ): 390.428 KiB Tracking/Track
  • DQMHistoSizes: changed ( 10024.0,... ): 97.796 KiB HLT/EGM
  • DQMHistoSizes: changed ( 10024.0,... ): 0.189 KiB Tracking/TrackConversion
  • DQMHistoSizes: changed ( 10024.0,... ): 0.183 KiB Tracking/TrackGsf
  • DQMHistoSizes: changed ( 20034.0,... ): 195.620 KiB Tracking/TrackTPEtaGreater2p7
  • Checked 147 log files, 16 edm output root files, 34 DQM output files

@jfernan2
Copy link
Contributor

@emiglior the PR is adding plots which depend on the workflow. In some cases they are empty (mainly in HLT folder), in others just modifications. For example I don't understand why there are RPC changes in wf12434 but perhaps this is expected.
Can you please check the changes and evaluate if you expect them?
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_1_X_2020-02-17-2300+73c518/35117/dqm-histo-comparison-summary.html
Thanks

@emiglior
Copy link
Contributor Author

emiglior commented Feb 18, 2020

Maybe I am too naive, but it seems to me that many of the differences are in the decimals of the mean and RMS of the trend plots (but correct me if I am misunderstanding something...)

Screen Shot 2020-02-18 at 17 30 18

About empty plots, I think they appear in workflows with t0o few events (added histos are trend plots obtained projecting 2d histos in phi-bins and fitting them to extract sigma and mean).

@jfernan2
Copy link
Contributor

Maybe I am too naive, but it seems to me that many of the differences are in the decimals of the mean and RMS of the trend plots (but correct me if I am misunderstanding something...)

No, you are NOT too naive, that's the case for many of the differences, but not always:
https://tinyurl.com/vlypkdv
and others should not be changing with this PR like https://tinyurl.com/w6ogdxj

About empty plots, I think they appear in workflows with t0o few events (added histos are trend plots obtained projecting 2d histos in phi-bins and fitting them to extract sigma and mean).

OK, but did you expect to be changing plots in HLT folder? E.g. https://tinyurl.com/to8xmm5

@mtosi
Copy link
Contributor

mtosi commented Feb 19, 2020

ciao,
sorry for jumping in
I think the "differences" spotted by the statistical test are false positive, driven by the threshold chosen
while, the appearance of plots under the HLT directory is expected, because the MTV is used for the validation of the tracking used in both the PF reconstruction and the GSF one at HLT
for instance, looking at the standard relval in recent release one can see all the rest of plots produced by the MTV relval GUI

hope it helps
have a nice day

@jfernan2
Copy link
Contributor

@mtosi I am not sure what threshold do you mean, there is no statistical test, just a bin by bin comparison of the plots produced with the PR w.r.t. those in baseline.

@mtosi
Copy link
Contributor

mtosi commented Feb 19, 2020

i thought you were using the KS or the chi2

@jfernan2
Copy link
Contributor

About " the appearance of plots under the HLT directory is expected, because the MTV is used for the validation of the tracking used in both the PF reconstruction and the GSF one at HLT", it was not clear to me since the PR description only mentions the MTV, but no comment about the GSF at HLT, hence my question

@jfernan2
Copy link
Contributor

+1
Tracking confirmed and agreed on changes

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

@silviodonato
Copy link
Contributor

+1

@cmsbuild cmsbuild merged commit 17e53db into cms-sw:master Feb 24, 2020
@emiglior emiglior deleted the MTV_residuals_vs_phi branch March 18, 2020 10:58
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

5 participants