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

2D Maps of Impact Parameters for TkDQM #33693

Merged
merged 2 commits into from May 12, 2021

Conversation

dmeuser
Copy link
Contributor

@dmeuser dmeuser commented May 11, 2021

PR description:

We added a total of four 2D histograms showing the mean Impact Parameters as a function of track eta and phi for TkDQM. For each of the following track selections:

  • PV track with pT>1 GeV
  • PV track with pT>10 GeV

one histogram for dxy and one for dz is added.

PR validation:

The changes have been tested by running runTheMatrix.py -l 10024, which runs without any failed steps and produces the desired additional plots.
Here is a screenshot of the output DQM file showing one of the four maps:

if this PR is a backport please specify the original PR and why you need to backport that PR:

This PR is not a backport.

cc:
@mmusich, @arossi83, @sroychow

@cmsbuild
Copy link
Contributor

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33693/22605

  • This PR adds an extra 16KB to repository

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

@cmsbuild
Copy link
Contributor

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33693/22607

  • This PR adds an extra 20KB to repository

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @dmeuser for master.

It involves the following packages:

DQMOffline/RecoB

@andrius-k, @kmaeshima, @ErnestaP, @ahmad3213, @cmsbuild, @jfernan2, @rvenditti can you please review it and eventually sign? Thanks.
@ferencek, @JyothsnaKomaragiri, @emilbols, @andrzejnovak, @rociovilar 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

@jfernan2
Copy link
Contributor

please test

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0c3da9/14998/summary.html
COMMIT: bc72091
CMSSW: CMSSW_12_0_X_2021-05-10-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33693/14998/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: 5 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2663174
  • DQMHistoTests: Total failures: 12
  • DQMHistoTests: Total nulls: 1
  • DQMHistoTests: Total successes: 2663139
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 146.957 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 1000.0,... ): 4.520 KiB OfflinePV/Alignment
  • DQMHistoSizes: changed ( 11634.0,... ): 13.844 KiB HLT/Vertexing
  • DQMHistoSizes: changed ( 23234.0,... ): 4.957 KiB OfflinePV/Alignment
  • 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

@jfernan2
Copy link
Contributor

@dmeuser

Plots on folders:
HLT / Vertexing / hltTrimmedPixelVertices / Alignment

HLT / Vertexing / hltPixelVertices / Alignment

seem empty, is this expected?

@mmusich
Copy link
Contributor

mmusich commented May 11, 2021

Plots on folders:
HLT / Vertexing / hltTrimmedPixelVertices / Alignment
HLT / Vertexing / hltPixelVertices / Alignment
seem empty, is this expected?

@jfernan2
are we running a (not Fake) HLT menu in these workflows? If not, yes it's expected

@mmusich
Copy link
Contributor

mmusich commented May 11, 2021

@jfernan2
I think this source is misconfigured (in general) in the HLT case and not only for these plots.
In fact, plots in that directory are all empty e.g. in a recent (CMSSW_11_3_0_pre6) RelVal campaign (see here).
I can try to see why that happens (while it is certainly responsibility of the HLT DQM people to make sure their plots are filled), but it has in fact nothing to do with the changes proposed here.

@mmusich
Copy link
Contributor

mmusich commented May 11, 2021

@jfernan2 after quick exchange with @mtosi we've understood the source of the problem:
In this module:

for (reco::Vertex::trackRef_iterator t = v.tracks_begin(); t != v.tracks_end(); t++) {
bool isHighPurity = (**t).quality(reco::TrackBase::highPurity);
if (!isHighPurity)
continue;

there is a requirement for the highPurity flag to be present for the track to pass the selection.
The HP flag is not available for HLT tracks, therefore the if statement at line 401 is always failed.
We should make this requirement optional and configure it to be transparent for HLT.
Let us know if this is something we can follow-up in a separate PR.

@jfernan2
Copy link
Contributor

Thanks @mmusich
In this case my understanding is that Tracking HLT DQM and Validation Code Developer(s) are responsible for these plots(?) There is no HLT DQM per sé, but HLT DQM developers linked to the subsystems:
in this case cms-tsg-steam-valdqm-developers-trk e-group at
https://twiki.cern.ch/twiki/bin/viewauth/CMS/DQMContacts#Tracking

Yes, please, if this could be fixed, even if in a separate PR, to avoid having expensive (in terms of memory) 3D plots, it would be nice. Unless you consider they are not actualy needed (I am not sure if the plots you point out as empty in the Validation campaigns are exactly the same as those these PR is adding) in which case, I'd suggest droping them, in both cases.

Thank you very much

@mmusich
Copy link
Contributor

mmusich commented May 11, 2021

@jfernan2

in this case cms-tsg-steam-valdqm-developers-trk e-group

yes, these are the people responsible.

Unless you consider they are not actualy needed

They are needed to monitor alignment @ HLT

(I am not sure if the plots you point out as empty in the Validation campaigns are exactly the same as those these PR is adding)

of course they are not the same, as the plots this PR is adding are new and could not be produced in Validation campaign done with an old release.
The point is that the same module is producing both sets of plots (the ones currently in the Validation campaign AND the ones added here), all empty because of the reasons I gave above #33693 (comment).

in a separate PR,

I would prefer to keep the fix separate.
So can this PR be approved?
Thanks

@mmusich
Copy link
Contributor

mmusich commented May 11, 2021

Follow-up PR at #33696.

@mmusich
Copy link
Contributor

mmusich commented May 11, 2021

@cmsbuild please test with #33696

@cmsbuild
Copy link
Contributor

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0c3da9/15008/summary.html
COMMIT: bc72091
CMSSW: CMSSW_12_0_X_2021-05-11-1100/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33693/15008/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: 0 differences found in the comparisons
  • DQMHistoTests: Total files compared: 37
  • DQMHistoTests: Total histograms compared: 2663174
  • DQMHistoTests: Total failures: 73
  • DQMHistoTests: Total nulls: 0
  • DQMHistoTests: Total successes: 2663079
  • DQMHistoTests: Total skipped: 22
  • DQMHistoTests: Total Missing objects: 0
  • DQMHistoSizes: Histogram memory added: 146.954 KiB( 36 files compared)
  • DQMHistoSizes: changed ( 1000.0,... ): 4.520 KiB OfflinePV/Alignment
  • DQMHistoSizes: changed ( 11634.0,... ): 13.844 KiB HLT/Vertexing
  • DQMHistoSizes: changed ( 23234.0,... ): 4.957 KiB OfflinePV/Alignment
  • Checked 155 log files, 37 edm output root files, 37 DQM output files
  • TriggerResults: no differences found

@mmusich
Copy link
Contributor

mmusich commented May 12, 2021

@jfernan2 when this PR is tested with #33696 the 2D plots in HLT / Vertexing / hltPixelVertices / Alignment are filled.

image

@jfernan2
Copy link
Contributor

+1
Thank you very much

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

@silviodonato
Copy link
Contributor

+1
#33696 is already merged

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