add MTD related hits plots #27682

mtosi · 2019-08-02T15:24:37Z

PR description:

add 3 new histograms to the MTV for having the MTD hits profile vs eta

@makortel do you think we should add a flag for the phase2 (so that we add those plots only for the relevant workflows) ?

cmsbuild · 2019-08-02T15:25:08Z

The code-checks are being triggered in jenkins.

mtosi · 2019-08-02T15:26:43Z

@lgray @pmeridian

cmsbuild · 2019-08-02T15:32:49Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27682/11270

This PR adds an extra 32KB to repository
There are other open Pull requests which might conflict with changes you have proposed:
- File Validation/RecoTrack/interface/MTVHistoProducerAlgoForTracker.h modified in PR(s): MTV phase2 high eta #27653
- File Validation/RecoTrack/src/MTVHistoProducerAlgoForTracker.cc modified in PR(s): MTV phase2 high eta #27653

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

code-format:
https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27682/11270/code-format.patch
e.g. curl https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27682/11270/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

makortel · 2019-08-02T15:37:02Z

do you think we should add a flag for the phase2 (so that we add those plots only for the relevant workflows) ?

Hmm, I'm not sure. It's a bit of a hassle, but would save some memory for non-phase2 workflows. If more MTD plots are foreseen, the flag would start to be more useful.

mtosi · 2019-08-02T15:55:49Z

well, in view of a more 4D tracking (and vertexing), I would expect more plots which are relevant only with the MTD detectors

but, this will not happen in the short term, I’m afraid

if you agree, I’ll add a flag
or are there more brilliant ways ?

makortel · 2019-08-02T15:57:59Z

if you agree, I’ll add a flag
or are there more brilliant ways ?

Sounds good, a configuration flag customized with an era is the best way.

mtosi · 2019-08-02T17:24:08Z

thanks for the confirmation mia

…

On 2 Aug 2019, at 17:58, Matti Kortelainen ***@***.***> wrote: if you agree, I’ll add a flag or are there more brilliant ways ? Sounds good, a configuration flag customized with an era is the best way. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

fabiocos · 2019-09-10T08:04:44Z

@mtosi what are the plans for this PR? It looks dormant...

mtosi · 2019-09-10T09:50:59Z

@fabiocos , thanks for the message
apologies for the delay, but I'm just back from Holidays ...

fix

jfernan2 · 2019-09-17T14:21:47Z

please test

cmsbuild · 2019-09-17T14:22:41Z

The tests are being triggered in jenkins.
https://cmssdt.cern.ch/jenkins/job/ib-run-pr-tests/2546/console Started: 2019/09/17 16:23

cmsbuild · 2019-09-17T16:22:48Z

+1
Tested at: e5c8daf
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b2133b/2546/summary.html

cmsbuild · 2019-09-17T16:22:52Z

Comparison job queued.

cmsbuild · 2019-09-17T18:37:26Z

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-b2133b/2546/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: 2957336
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2956994
DQMHistoTests: Total skipped: 341
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 607.748 KiB( 33 files compared)
DQMHistoSizes: changed ( 20434.0,... ): 96.938 KiB Tracking/Track
DQMHistoSizes: changed ( 20434.0,... ): 18.996 KiB Tracking/TrackBuilding
DQMHistoSizes: changed ( 20434.0,... ): 9.686 KiB Tracking/TrackFromPVAllTP
DQMHistoSizes: changed ( 20434.0,... ): 9.627 KiB Tracking/TrackFromPV
DQMHistoSizes: changed ( 20434.0,... ): 9.551 KiB Tracking/TrackConversion
DQMHistoSizes: changed ( 20434.0,... ): 4.790 KiB Tracking/TrackAllTPEffic
DQMHistoSizes: changed ( 20434.0,... ): 2.350 KiB Tracking/TrackGsf
Checked 145 log files, 15 edm output root files, 34 DQM output files

jfernan2 · 2019-09-18T06:53:45Z

@mtosi are you aware that you add 3 plots X 63 folders = 189 histograms? And that in some cases they are empty? It was not clear to me from your PR description
https://cmssdt.cern.ch/SDT/jenkins-artifacts/baseLineComparisons/CMSSW_11_0_X_2019-09-16-2300+b2133b/33509/dqm-histo-comparison-summary.html

mtosi · 2019-09-18T11:51:05Z

ciao
thanks for the comment, indeed it is expected and meant to validate / monitor the hits from the MTD detector. they are not used in the current pattern recognition, but the MTD and TRK POG are planning to make use of them as well

jfernan2 · 2019-09-18T14:11:20Z

+1

cmsbuild · 2019-09-18T14:11:49Z

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

fabiocos · 2019-09-18T20:06:36Z

Validation/RecoTrack/src/MTVHistoProducerAlgoForTracker.cc

+    //  const auto mtdHits = track.hitPattern().numberOfValidTimingHits();
+    const auto btlHits = track.hitPattern().numberOfValidTimingBTLHits();
+    const auto etlHits = track.hitPattern().numberOfValidTimingETLHits();
+    histograms.nMTDhits_vs_eta[count].fill(eta, btlHits + etlHits);


@mtosi sorry, the 3 histograms are identical in definition, just filled with either btl, etl or both. At the least the last histogram is redundant (or the first two). Could you please clarify the rationale behind this choice, that ahs an impact in memory?

mtosi · 2019-09-18T21:37:43Z

as always, we make plots for subsystem and for the whole
(as we do for the 4 strip subsystem, and and the whole strip detector)

fabiocos · 2019-09-19T13:35:37Z

@mtosi @jfernan2 in case we start to be compressed in memory a choice to be reconsidered...

mtosi · 2019-09-19T13:42:42Z

the only thing i can add is that if one wants to be independent, and check the hitPattern filling, then we should use the hitPattern directly instead of passing through the sum point taken, thanks mia

…

On 19 Sep 2019, at 15:35, Fabio Cossutti ***@***.***> wrote: @mtosi @jfernan2 in case we start to be compressed in memory a choice to be reconsidered... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

fioriNTU · 2019-09-19T13:51:31Z

@mtosi @fabiocos one possible workaround would be to fill etl and btl plots in step1 and then sum up them in Harvesting. This would save 1/3 of the memory added. However the increase in memory of about 600 KB seems completely acceptable, so we can safely integrate it for the time being.

fabiocos · 2019-09-19T20:22:49Z

+1

cmsbuild added this to the CMSSW_11_0_X milestone Aug 2, 2019

cmsbuild added code-checks-pending comparison-pending dqm-pending orp-pending pending-signatures tests-pending labels Aug 2, 2019

cmsbuild added code-checks-rejected and removed code-checks-pending labels Aug 2, 2019

cmsbuild mentioned this pull request Aug 12, 2019

MTV phase2 high eta #27653

Merged

cmsbuild mentioned this pull request Sep 2, 2019

Add energy regression and particle identification to RecoHGCal/TICL. #27634

Closed

mtosi added 2 commits September 12, 2019 14:35

add MTD related hits plots

3adc41d

fix

add flag for handling the new plots

eb9bf3e

mtosi force-pushed the MTV_addMTDhits branch from 2882747 to eb9bf3e Compare September 12, 2019 12:36

cmsbuild added alca-pending analysis-pending code-checks-pending core-pending daq-pending db-pending and removed code-checks-rejected labels Sep 12, 2019

cmsbuild added code-checks-approved and removed code-checks-pending labels Sep 17, 2019

cmsbuild added tests-started and removed tests-pending labels Sep 17, 2019

cmsbuild added tests-approved and removed tests-started labels Sep 17, 2019

cmsbuild added comparison-available and removed comparison-pending labels Sep 17, 2019

cmsbuild added dqm-approved fully-signed and removed dqm-pending pending-signatures labels Sep 18, 2019

fabiocos reviewed Sep 18, 2019

View reviewed changes

cmsbuild added orp-approved and removed orp-pending labels Sep 19, 2019

cmsbuild merged commit dfa1c73 into cms-sw:master Sep 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add MTD related hits plots #27682

add MTD related hits plots #27682

mtosi commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

mtosi commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

makortel commented Aug 2, 2019

mtosi commented Aug 2, 2019

makortel commented Aug 2, 2019

mtosi commented Aug 2, 2019 via email

fabiocos commented Sep 10, 2019

mtosi commented Sep 10, 2019

jfernan2 commented Sep 17, 2019

cmsbuild commented Sep 17, 2019 •

edited

cmsbuild commented Sep 17, 2019

cmsbuild commented Sep 17, 2019

cmsbuild commented Sep 17, 2019

jfernan2 commented Sep 18, 2019

mtosi commented Sep 18, 2019

jfernan2 commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

fabiocos Sep 18, 2019

mtosi commented Sep 18, 2019

fabiocos commented Sep 19, 2019

mtosi commented Sep 19, 2019 via email

fioriNTU commented Sep 19, 2019

fabiocos commented Sep 19, 2019

add MTD related hits plots #27682

add MTD related hits plots #27682

Conversation

mtosi commented Aug 2, 2019

PR description:

cmsbuild commented Aug 2, 2019

mtosi commented Aug 2, 2019

cmsbuild commented Aug 2, 2019

makortel commented Aug 2, 2019

mtosi commented Aug 2, 2019

makortel commented Aug 2, 2019

mtosi commented Aug 2, 2019 via email

fabiocos commented Sep 10, 2019

mtosi commented Sep 10, 2019

jfernan2 commented Sep 17, 2019

cmsbuild commented Sep 17, 2019 • edited

cmsbuild commented Sep 17, 2019

cmsbuild commented Sep 17, 2019

cmsbuild commented Sep 17, 2019

jfernan2 commented Sep 18, 2019

mtosi commented Sep 18, 2019

jfernan2 commented Sep 18, 2019

cmsbuild commented Sep 18, 2019

fabiocos Sep 18, 2019

Choose a reason for hiding this comment

mtosi commented Sep 18, 2019

fabiocos commented Sep 19, 2019

mtosi commented Sep 19, 2019 via email

fioriNTU commented Sep 19, 2019

fabiocos commented Sep 19, 2019

cmsbuild commented Sep 17, 2019 •

edited