TkDQM: add rechit harvester module for IT #33863

sroychow · 2021-05-27T12:38:30Z

PR description:

This PR introduces the following:-

In the DQM step, (rechit-simhit) residuals are plotted as a function of abs(eta) and phi for each layer and ring. This is done both for the IT and OT(OT is still only in standalone configuration)
A new module is introduced for Harvesting, which does the fits of the residual 2D histos of the previous point to make plots of mean residual and resolution from as a function of eta-phi
For finding the closest simhit for rechits, and clusters - squared distance is used
A new harvesting util is also added.
The harvester module can also be configured to run for OT but it can only be used in standalone mode now.

PR validation:

Checked with wf 23234.0

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

NA
cc @emiglior @mmusich @tsusa

cmsbuild · 2021-05-27T12:45:00Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33863/22893

This PR adds an extra 40KB to repository

cmsbuild · 2021-05-27T12:45:25Z

A new Pull Request was created by @sroychow (Suvankar Roy Chowdhury) for master.

It involves the following packages:

DQM/SiTrackerPhase2
Validation/SiTrackerPhase2V

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

mmusich · 2021-05-27T12:45:29Z

@cmsbuild, please test

cmsbuild · 2021-05-27T15:40:36Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e30d1a/15352/summary.html
COMMIT: d6fb725
CMSSW: CMSSW_12_0_X_2021-05-26-2300/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmssw/33863/15352/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: 8 differences found in the comparisons
DQMHistoTests: Total files compared: 37
DQMHistoTests: Total histograms compared: 2650486
DQMHistoTests: Total failures: 2279
DQMHistoTests: Total nulls: 398
DQMHistoTests: Total successes: 2647787
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 11488.088 KiB( 36 files compared)
DQMHistoSizes: changed ( 23234.0,... ): 587.971 KiB TrackerPhase2ITTrackingRecHitV/EndCap_Side1
DQMHistoSizes: changed ( 23234.0,... ): 587.971 KiB TrackerPhase2ITTrackingRecHitV/EndCap_Side2
DQMHistoSizes: changed ( 23234.0,... ): 587.268 KiB TrackerPhase2ITRecHitV/EndCap_Side2
DQMHistoSizes: changed ( 23234.0,... ): 587.268 KiB TrackerPhase2ITRecHitV/EndCap_Side1
DQMHistoSizes: changed ( 23234.0,... ): 260.930 KiB TrackerPhase2ITTrackingRecHitV/Barrel
DQMHistoSizes: changed ( 23234.0,... ): 260.617 KiB TrackerPhase2ITRecHitV/Barrel
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

mmusich

sorry for the long delay @sroychow.
I have a minor comment on possible code reduction.
One question: is there any reason why Phase2OTValidateTrackingRecHit cannot run in the standard production sequence? Seems it's relegated to the standalone workflow now.

mmusich · 2021-06-09T08:10:56Z

Validation/SiTrackerPhase2V/plugins/Phase2ITValidateRecHit.cc

      LocalPoint lp = rechit.localPosition();
      for (const auto& simHitCol : simHits) {
        for (const auto& simhitIt : *simHitCol) {
          if (detId.rawId() != simhitIt.detUnitId())
            continue;
          for (const auto& mId : matchedId) {
            if (simhitIt.trackId() == mId.first) {
-              if (!simhitClosest || abs(simhitIt.localPosition().x() - lp.x()) < minx) {
-                minx = abs(simhitIt.localPosition().x() - lp.x());
+              float dx = simhitIt.localPosition().x() - lp.x();


I think this whole block of finding the closest simhit is repeated in several classes. Perhaps can be moved to the base or even higher up.

mmusich · 2021-06-09T08:21:57Z

Validation/SiTrackerPhase2V/plugins/Phase2ITRecHitHarvester.cc

+      float minfit = histoY->GetMean() - histoY->GetRMS();
+      float maxfit = histoY->GetMean() + histoY->GetRMS();
+
+      TF1* fitFcn = new TF1(TString("g") + histo->GetName() + iString, "gaus", minfit, maxfit);


wondering if one could use a smart pointer here?

Yes will do.

sroychow · 2021-06-09T09:54:26Z

@mmusich on the Phase2OTValidateTrackingRecHit, yes it should be moved to the standard sequence. I will update the config.
Concerning the simhit,rechit matching - yes this should be optimized. I shall do it in a follow up PR if you are okay.

cmsbuild · 2021-06-16T15:54:08Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-33863/23365

This PR adds an extra 24KB to repository

cmsbuild · 2021-06-16T15:54:27Z

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

jfernan2 · 2021-06-16T15:56:11Z

+1
Last commit is cosmetic

cmsbuild · 2021-06-16T15:56:35Z

This pull request is fully signed and it will be integrated in one of the next master IBs after it passes the integration tests. 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)

cmsbuild · 2021-06-16T18:45:12Z

+1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-e30d1a/16018/summary.html
COMMIT: 8843f55
CMSSW: CMSSW_12_0_X_2021-06-16-0900/slc7_amd64_gcc900
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week1/cms-sw/cmssw/33863/16018/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: 38
DQMHistoTests: Total histograms compared: 2862520
DQMHistoTests: Total failures: 1
DQMHistoTests: Total nulls: 352
DQMHistoTests: Total successes: 2862145
DQMHistoTests: Total skipped: 22
DQMHistoTests: Total Missing objects: 0
DQMHistoSizes: Histogram memory added: 66615.752 KiB( 37 files compared)
DQMHistoSizes: changed ( 23234.0,... ): 6267.833 KiB TrackerPhase2OTTrackingRecHitV/EndCap_Side2
DQMHistoSizes: changed ( 23234.0,... ): 6267.833 KiB TrackerPhase2OTTrackingRecHitV/EndCap_Side1
DQMHistoSizes: changed ( 23234.0,... ): 1246.248 KiB TrackerPhase2OTTrackingRecHitV/Barrel
DQMHistoSizes: changed ( 23234.0,... ): 587.971 KiB TrackerPhase2ITTrackingRecHitV/EndCap_Side1
DQMHistoSizes: changed ( 23234.0,... ): 587.971 KiB TrackerPhase2ITTrackingRecHitV/EndCap_Side2
DQMHistoSizes: changed ( 23234.0,... ): 587.268 KiB TrackerPhase2ITRecHitV/EndCap_Side1
DQMHistoSizes: changed ( 23234.0,... ): 587.268 KiB TrackerPhase2ITRecHitV/EndCap_Side2
DQMHistoSizes: changed ( 23234.0,... ): 260.930 KiB TrackerPhase2ITTrackingRecHitV/Barrel
DQMHistoSizes: changed ( 23234.0,... ): 260.617 KiB TrackerPhase2ITRecHitV/Barrel
Checked 160 log files, 37 edm output root files, 38 DQM output files
TriggerResults: no differences found

qliphy · 2021-06-17T00:10:22Z

+1

qliphy · 2021-06-18T00:28:41Z

@sroychow There is a reval error in IB after this PR was merged. Not sure this is a glitch, to be checked with next IB, but it would be nice if you can have a look:

https://cmssdt.cern.ch/SDT/html/cmssdt-ib/#/relVal/CMSSW_12_0/2021-06-17-1100?selectedArchs=slc7_amd64_gcc900&selectedFlavors=X&selectedStatus=failed
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-06-17-1100/pyRelValMatrixLogs/run/23234.9_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step3_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log#/173-173

Module: Phase2OTValidateTrackingRecHit:trackingRechitValidOT (crashed)
Module: Phase2ITValidateTrackingRecHit:trackingRechitValidIT
Module: none
Module: TrackstersProducer:ticlTrackstersEM
A fatal system signal has occurred: segmentation violation

sroychow · 2021-06-18T05:44:13Z

@qliphy thanks for pointing it out. I am investigating it.

mmusich · 2021-06-18T06:07:34Z

@sroychow @qliphy the affected workflow appears to make use of VectorHits, so the error looks genuine. The input OT Rechit collection ought to be changed, but given we're planning to use a different module I suggest to make use of the dedicated modifier and exclude Phase2OTValidateTrackingRecHit:trackingRechitValidOT from execution.

sroychow · 2021-06-18T07:00:18Z

@mmusich @qliphy yes I have addressed the issue in a new PR excluding the OT validation and harvesting module from the standard sequence with the vectorHits modifier.

mrodozov · 2021-06-18T08:36:05Z

@sroychow There is a reval error in IB after this PR was merged. Not sure this is a glitch, to be checked with next IB, but it would be nice if you can have a look:

https://cmssdt.cern.ch/SDT/html/cmssdt-ib/#/relVal/CMSSW_12_0/2021-06-17-1100?selectedArchs=slc7_amd64_gcc900&selectedFlavors=X&selectedStatus=failed
https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc900/CMSSW_12_0_X_2021-06-17-1100/pyRelValMatrixLogs/run/23234.9_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step3_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log#/173-173

Module: Phase2OTValidateTrackingRecHit:trackingRechitValidOT (crashed)
Module: Phase2ITValidateTrackingRecHit:trackingRechitValidIT
Module: none
Module: TrackstersProducer:ticlTrackstersEM
A fatal system signal has occurred: segmentation violation

it's not a glitch it's in all latest IBs

mmusich · 2021-06-18T08:43:48Z

@mrodozov I think you missed my message above #33863 (comment) ...

qliphy · 2021-06-18T08:44:10Z

it's not a glitch it's in all latest IBs

Right. @sroychow has made a fix #34175

cmsbuild added this to the CMSSW_12_0_X milestone May 27, 2021

cmsbuild added code-checks-pending dqm-pending orp-pending pending-signatures tests-pending labels May 27, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels May 27, 2021

cmsbuild added tests-started and removed tests-pending labels May 27, 2021

cmsbuild added tests-approved and removed tests-started labels May 27, 2021

mmusich reviewed Jun 9, 2021

View reviewed changes

sroychow added 7 commits June 10, 2021 09:01

add rechit resoultion harvester

a5506ec

move to square dist when matching instead of sqrt

03283a7

fold in eta and add harvesting util

2864e63

apply code checks

1efb9d7

apply code format

3f4978b

fix range bug

7d2a806

add tracking rechit valid to central sequence;update entry threshold

f261458

sroychow force-pushed the rechitharvestph2tkv branch from d6fb725 to f261458 Compare June 11, 2021 07:36

cmsbuild added code-checks-pending tests-pending and removed code-checks-approved tests-approved labels Jun 11, 2021

cmsbuild added code-checks-pending tests-pending and removed tests-approved labels Jun 16, 2021

cmsbuild added code-checks-approved and removed code-checks-pending labels Jun 16, 2021

cmsbuild added dqm-approved fully-signed tests-started and removed dqm-pending pending-signatures tests-pending labels Jun 16, 2021

cmsbuild added tests-approved and removed tests-started labels Jun 16, 2021

cmsbuild added orp-approved and removed orp-pending labels Jun 17, 2021

cmsbuild merged commit 72459f6 into cms-sw:master Jun 17, 2021

sroychow mentioned this pull request Jun 18, 2021

TkDQM: remove OT validation module for vector hits workflow #34175

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TkDQM: add rechit harvester module for IT #33863

TkDQM: add rechit harvester module for IT #33863

sroychow commented May 27, 2021

cmsbuild commented May 27, 2021

cmsbuild commented May 27, 2021

mmusich commented May 27, 2021

cmsbuild commented May 27, 2021

mmusich left a comment

mmusich Jun 9, 2021

mmusich Jun 9, 2021

sroychow Jun 9, 2021

sroychow commented Jun 9, 2021

cmsbuild commented Jun 16, 2021

cmsbuild commented Jun 16, 2021

jfernan2 commented Jun 16, 2021

cmsbuild commented Jun 16, 2021

cmsbuild commented Jun 16, 2021

qliphy commented Jun 17, 2021

qliphy commented Jun 18, 2021

sroychow commented Jun 18, 2021

mmusich commented Jun 18, 2021 •

edited

sroychow commented Jun 18, 2021

mrodozov commented Jun 18, 2021

mmusich commented Jun 18, 2021

qliphy commented Jun 18, 2021

TkDQM: add rechit harvester module for IT #33863

TkDQM: add rechit harvester module for IT #33863

Conversation

sroychow commented May 27, 2021

PR description:

PR validation:

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

cmsbuild commented May 27, 2021

cmsbuild commented May 27, 2021

mmusich commented May 27, 2021

cmsbuild commented May 27, 2021

Comparison Summary

mmusich left a comment

Choose a reason for hiding this comment

mmusich Jun 9, 2021

Choose a reason for hiding this comment

mmusich Jun 9, 2021

Choose a reason for hiding this comment

sroychow Jun 9, 2021

Choose a reason for hiding this comment

sroychow commented Jun 9, 2021

cmsbuild commented Jun 16, 2021

cmsbuild commented Jun 16, 2021

jfernan2 commented Jun 16, 2021

cmsbuild commented Jun 16, 2021

cmsbuild commented Jun 16, 2021

Comparison Summary

qliphy commented Jun 17, 2021

qliphy commented Jun 18, 2021

sroychow commented Jun 18, 2021

mmusich commented Jun 18, 2021 • edited

sroychow commented Jun 18, 2021

mrodozov commented Jun 18, 2021

mmusich commented Jun 18, 2021

qliphy commented Jun 18, 2021

mmusich commented Jun 18, 2021 •

edited