modified merging scheme and tighter outlier suppression in primary vertex reconstruction #33341

werdmann · 2021-04-06T14:35:12Z

PR description:

The 3d clustering is modified slightly in order to reduce splitting and fake rates for Run 3. This addresses fake-rate and splitting related observations made in Run 2 data. Details and results can be found in
https://indico.cern.ch/event/1019373/contributions/4293207/attachments/2217496/3754437/2021-03-29-tuning.pdf
and
https://indico.cern.ch/event/1023338/contributions/4298206/attachments/2219781/3758715/2021-04-01-pvtuning.pdf
The code changes are

remove the Tc control for merging. The net effect of this feature is an increased fake rate that does not justify the small gain in efficiency. Updates of Tc are now initiated by the split() routine.
the proporionality constant of the outlier suppression term is increased from 1/#tracks to 1/#clusters. The value of dzCutOff can now be seen as the soft z-cut-off for an average cluster.

Small changes in the number and composition of reconstructed vertices are expected.

PR validation:

The code was tested with several 11_3_0_pre4 relval samples with the results reported in the presentations mentioned above.
runTheMatrix with this PR code on top of CMSSW_11_3_X_2021-04-05-2300 produces some errors, however, I can not see any connection to the vertex reconstruction ( 37 34 26 18 14 4 1 1 1 tests passed, 0 2 7 0 0 0 0 0 0 failed )

…roportional to the average cluster mass

cmsbuild · 2021-04-06T14:41:19Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33341/21916

This PR adds an extra 20KB to repository

cmsbuild · 2021-04-06T14:41:44Z

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

It involves the following packages:

RecoVertex/PrimaryVertexProducer

@perrotta, @jpata, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks.
@makortel, @GiacomoSguazzoni, @JanFSchulte, @rovere, @VinInn, @ebrondol, @mtosi, @dgulhan 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

perrotta · 2021-04-07T08:53:44Z

please test

mtosi · 2021-04-07T10:01:18Z

@werdmann thanks for this PR, may I suggest updating the title and adding [PV reco] or something like that ?

thanks a lot !

cmsbuild · 2021-04-07T12:20:01Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d2e7ae/14054/summary.html
COMMIT: 71b1f6f
CMSSW: CMSSW_11_3_X_2021-04-06-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33341/14054/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: 11193 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2640868
DQMHistoTests: Total failures: 56327
DQMHistoTests: Total nulls: 1
DQMHistoTests: Total successes: 2584518
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: -0.004 KiB( 36 files compared)
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

perrotta · 2021-04-09T06:56:06Z

RecoVertex/PrimaryVertexProducer/src/DAClusterizerInZ_vect.cc

@@ -483,31 +483,26 @@ bool DAClusterizerInZ_vect::merge(vertex_t& y, track_t& tks, double& beta) const
  for (unsigned int ik = 0; ik < critical.size(); ik++) {
    unsigned int k = critical[ik].second;
    double rho = y.rho[k] + y.rho[k + 1];
-    double swE = y.swE[k] + y.swE[k + 1] - y.rho[k] * y.rho[k + 1] / rho * std::pow(y.zvtx[k + 1] - y.zvtx[k], 2);
-    double Tc = 2 * swE / (y.sw[k] + y.sw[k + 1]);


Since you are removing the local variable Tc, please also remove it from the cout at L490

thanks for catching. I'll also modify the "dump" routine, which is only used when DEBUG is enabled, to print meaningful Tc.

perrotta · 2021-04-09T07:16:40Z

@werdmann thanks for this PR, may I suggest updating the title and adding [PV reco] or something like that ?

thanks a lot !

@mtosi can I consider this as an approval of the update of the algorithm from the Tracking POG?
As far as I can see, the outputs of the (low stat) jenkins tests do not show issues that can contradict what was presented and discussed in the POG

cmsbuild · 2021-04-09T08:17:56Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33341/21976

This PR adds an extra 24KB to repository

cmsbuild · 2021-04-09T08:18:19Z

Pull request #33341 was updated. @perrotta, @jpata, @cmsbuild, @slava77 can you please check and sign again.

perrotta · 2021-04-09T08:23:30Z

please test

cmsbuild · 2021-04-09T11:18:41Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-d2e7ae/14127/summary.html
COMMIT: 1107cea
CMSSW: CMSSW_11_3_X_2021-04-08-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33341/14127/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: 11789 differences found in the comparisons
DQMHistoTests: Total files compared: 38
DQMHistoTests: Total histograms compared: 2865506
DQMHistoTests: Total failures: 36674
DQMHistoTests: Total nulls: 0
DQMHistoTests: Total successes: 2828810
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 0.0 KiB( 37 files compared)
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

perrotta · 2021-04-09T14:50:55Z

+1

Small modifications to the algorithm, implemented as stated in the PR description.
Jenkins tests show overall a slight reduction in the number of reconstructed vertices (expected, with the reduction of fakes and splitting) and a few minor changes in all related quantities. Larger stat validation is available in the plots attached to the PR description.
Update discussed and approved in the tracking POG.

cmsbuild · 2021-04-09T14:51:16Z

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 · 2021-04-10T14:29:05Z

+1

remove Tc control for merging and make the outlier suppression term p…

71b1f6f

…roportional to the average cluster mass

cmsbuild added this to the CMSSW_11_3_X milestone Apr 6, 2021

cmsbuild added code-checks-pending orp-pending pending-signatures reconstruction-pending tests-pending labels Apr 6, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 6, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 7, 2021

werdmann changed the title ~~modified merging scheme and tighter outlier suppression~~ modified merging scheme and tighter outlier suppression in primary vertex reconstruction Apr 7, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 7, 2021

perrotta reviewed Apr 9, 2021

View reviewed changes

fix DEBUG mode code that involves Tc

1107cea

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Apr 9, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Apr 9, 2021

cmsbuild added tests-started and removed tests-pending labels Apr 9, 2021

cmsbuild added tests-approved and removed tests-started labels Apr 9, 2021

cmsbuild mentioned this pull request Apr 9, 2021

MultiCluster-CaloParticle associator #32941

Closed

cmsbuild added fully-signed reconstruction-approved and removed pending-signatures reconstruction-pending labels Apr 9, 2021

cmsbuild added orp-approved and removed orp-pending labels Apr 10, 2021

cmsbuild merged commit cd2c682 into cms-sw:master Apr 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modified merging scheme and tighter outlier suppression in primary vertex reconstruction #33341

modified merging scheme and tighter outlier suppression in primary vertex reconstruction #33341

werdmann commented Apr 6, 2021

cmsbuild commented Apr 6, 2021

cmsbuild commented Apr 6, 2021

perrotta commented Apr 7, 2021

mtosi commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

perrotta Apr 9, 2021

werdmann Apr 9, 2021

perrotta commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

perrotta commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

perrotta commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

silviodonato commented Apr 10, 2021

modified merging scheme and tighter outlier suppression in primary vertex reconstruction #33341

modified merging scheme and tighter outlier suppression in primary vertex reconstruction #33341

Conversation

werdmann commented Apr 6, 2021

PR description:

PR validation:

cmsbuild commented Apr 6, 2021

cmsbuild commented Apr 6, 2021

perrotta commented Apr 7, 2021

mtosi commented Apr 7, 2021

cmsbuild commented Apr 7, 2021

Comparison Summary

perrotta Apr 9, 2021

Choose a reason for hiding this comment

werdmann Apr 9, 2021

Choose a reason for hiding this comment

perrotta commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

perrotta commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

Comparison Summary

perrotta commented Apr 9, 2021

cmsbuild commented Apr 9, 2021

silviodonato commented Apr 10, 2021