Outer Tracker Phase2 DQM: additional stub validation plots #45475

brandiskip · 2024-07-16T21:58:56Z

PR description:

Histograms added for stub coordinates Δφ and Δz, stub Δbend, and stub efficiency vs tp pT
Proposal for validation plots
Presentation slides with final results

PR validation:

Added histograms examined and validated, showing sensible results as presented at the offline software meeting

ran:
scram build code-checks
scram build code-format

merging save_branch to get TTStub cfi file

cmsbuild · 2024-07-16T21:59:21Z

cms-bot internal usage

cmsbuild · 2024-07-16T22:01:28Z

+code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45475/40939

Found files with invalid states:
- DQM/SiTrackerPhase2/test/step3_pre4_inDQM.root:
  - Added: 4ab0c8b
  - Deleted: 98f89e8
- DQM/SiTrackerPhase2/test/DQM_V0001_R000000001__Global__CMSSW_X_Y_Z__RECO.root:
  - Added: 4ab0c8b
  - Deleted: 98f89e8

cmsbuild · 2024-07-16T22:01:50Z

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

It involves the following packages:

DQM/SiTrackerPhase2 (dqm)
Validation/SiTrackerPhase2V (dqm)

@antoniovagnerini, @cmsbuild, @nothingface0, @rvenditti, @syuvivida, @tjavaid can you please review it and eventually sign? Thanks.
@arossi83, @missirol, @mmusich, @sroychow this is something you requested to watch as well.
@antoniovilela, @mandrenguyen, @rappoccio, @sextonkennedy you are the release manager for this.

cms-bot commands are listed here

tomalin · 2024-07-18T09:29:30Z

In https://indico.cern.ch/event/1414546/contributions/5965160/attachments/2861743/5006999/DQM_Update_OfflineSW.pdf

Slide 4 shows stub resolution in z. Is this really the resolution in r-z? The modules measure z in the barrel, but r in the endcap.
Slide 7 gives no info about the stub efficiency for Pt < 2 GeV. There is probably a little efficiency in the range between 1.5 - 2 GeV, so it would be nice to see it.

sroychow · 2024-07-17T04:30:35Z

Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc

+
+  // z-resolution for PS modules
+  HistoName = "#Delta z Barrel PS modules";
+  z_res_isPS_barrel = iBooker.book1D(HistoName,


Suggest to use the utility function.

sroychow · 2024-07-17T04:41:23Z

Validation/SiTrackerPhase2V/python/Phase2OTValidateTTStub_cfi.py

@@ -0,0 +1,20 @@
+import FWCore.ParameterSet.Config as cms


Since you have the fillDescriptions method, this default config file will be automatically generated while compiling.

Any changes can be done via cff files later. see example for clusters

sroychow · 2024-07-17T04:41:51Z

Validation/SiTrackerPhase2V/python/Phase2OTValidateTrackingParticles_cfi.py

@@ -0,0 +1,134 @@
+import FWCore.ParameterSet.Config as cms


Similar comment

tomalin · 2024-07-18T09:43:53Z

Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc

+
+  // delta_phi hists
+  MonitorElement* phi_res_barrel;
+  MonitorElement* phi_res_barrel_L1;


Instead of declaring separately phi_res_barrel_L1, phi_res_barrel_L2 etc., which requires many lines of code, use an std::vector<MonitorElement*>. Then when booking and filling the histograms, you can use for loops to make the code much shorter.

cmsbuild · 2024-07-22T19:33:23Z

-code-checks

Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45475/40996

Found files with invalid states:
- DQM/SiTrackerPhase2/test/step3_pre4_inDQM.root:
  - Added: 4ab0c8b
  - Deleted: 98f89e8
- DQM/SiTrackerPhase2/test/DQM_V0001_R000000001__Global__CMSSW_X_Y_Z__RECO.root:
  - Added: 4ab0c8b
  - Deleted: 98f89e8

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-45475/40996/code-format.patch
e.g. curl -k https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-45475/40996/code-format.patch | patch -p1
You can also run scram build code-format to apply code format directly

tomalin · 2024-07-22T23:20:24Z

Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc

+  theTrackerGeom_ = &(iSetup.getData(geomToken_));
+  tTopo_ = &(iSetup.getData(tTopoToken_));
+
+  // Clear existing histograms


Where does the idea to clear the existing histograms come from? It may be correct, but I don't see simiilar things happening in beginRun of the other DQM packages. e.g. https://github.com/brandiskip/cmssw/blob/test_branch/Validation/SiTrackerPhase2V/plugins/Phase2OTValidateCluster.cc .

I kept getting a seg fault and someone pointed me to lines 116 and 117 found here: https://cmssdt.cern.ch/lxr/source/DQM/HLTEvF/plugins/TriggerBxVsOrbitMonitor.cc?v=CMSSW_14_1_X_2024-07-17-2300
and when I followed that format, it fixed my issues. Perhaps there is a better way. I will investigate further.

You need to set the vector size to 6 or whatever, since you refer to the elements later on. e.g. https://github.com/brandiskip/cmssw/blob/test_branch/Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc#L452 . But I don't understand why it needs resetting o 6 every run. Or why the call to clear is needed. If it were set to 6 in the constructor, resize would not be needed.

tomalin · 2024-07-22T23:25:29Z

Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc

+        layer = -1;
+      }
+
+      int isPSmodule = (topo.nrows() == 960) ? 1 : 0;


Having magic numbers in CMSSW code, such as 960 is forbidden.

tomalin · 2024-07-22T23:30:41Z

Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc

+  bend_res_bw_endcap_discs.clear();
+
+  // Resize vectors and set elements to nullptr
+  phi_res_barrel_layers.resize(6, nullptr);


Having magic numbers in the code like 5 or 6 is not allowed. At the very least these need to be named constants.

tomalin · 2024-07-22T23:38:03Z

Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc

+
+      // Ensure that the vectors are correctly assigned before use
+      if (isBarrel == 1) {
+          phi_res_discs = &phi_res_barrel_layers; // Pointing to barrel layers vector


What is going on here? If we're in the barrel, why is variable phi_res_discs being set?

tomalin · 2024-07-22T23:40:24Z

Validation/SiTrackerPhase2V/plugins/Phase2OTValidateTTStub.cc

+      }
+
+      // Fill the appropriate histogram based on layer/disc
+      if (layer >= 1 && layer <= 6) {


I can't understand what this part of hte code is doing. The comment suggests that its filling both layers and disks. But it requires layer <= 6, when there are only 5 disks. And it fills a variable alled bend_res_discs, which doesn't sound appropriate for the barrel.

tomalin · 2024-07-22T23:41:29Z

A general question. How have you chosen the level of detail that is best for the validation code? e.g. Histograms for barrel + endcap vs histograms for every single layer?

tomalin · 2024-07-22T23:43:18Z

General comments: All classes need a comment at the top explaining what they are for.

cmsbuild · 2024-08-27T08:09:52Z

Milestone for this pull request has been moved to CMSSW_14_2_X. Please open a backport if it should also go in to CMSSW_14_1_X.

antoniovilela · 2024-09-03T09:30:59Z

ping (to make bot change milestone)

sarafiorendi and others added 15 commits June 6, 2024 16:54

include validateTP in the sequence,small changes

19177d3

added new histos for stubs

3e6ad0e

created for stub histos

fc748d5

added comment to add to git

4546abb

removed print statement

429a00a

preparing to merge pre4

4ab0c8b

Merge branch 'save_branch' into test_branch

fbfdfd3

merging save_branch to get TTStub cfi file

added phi_res to fillHistogram

861c783

minor changes w/all histos correct

98f89e8

access bfield and c from Settings.h

65e2492

remove any L1track variables and collection

419c3b2

minor changes from code-checks and code-format

a44deb4

Revert unnecessary changes and restore original process options

25d9a1c

remove TH1LeadingTPPtRatio parameter

0722806

restore previously removed customizations

525f1d5

cmsbuild added this to the CMSSW_14_1_X milestone Jul 16, 2024

cmsbuild added dqm-pending pending-signatures tests-pending orp-pending code-checks-pending trk labels Jul 16, 2024

cmsbuild added code-checks-approved and removed code-checks-pending labels Jul 16, 2024

sroychow reviewed Jul 18, 2024

View reviewed changes

tomalin reviewed Jul 18, 2024

View reviewed changes

simplified histogram booking and filling using for loops

b96881f

cmsbuild added code-checks-pending and removed code-checks-approved labels Jul 22, 2024

cmsbuild added code-checks-rejected and removed code-checks-pending labels Jul 22, 2024

tomalin reviewed Jul 22, 2024

View reviewed changes

cmsbuild mentioned this pull request Jul 26, 2024

Move dictionaries (and some classes) defined in SimTracker/TrackTriggerAssociation and SimGeneral/TrackingAnalysis to more proper packages #45567

Merged

cmsbuild mentioned this pull request Aug 16, 2024

Parametrized magnetic field in alpaka #44360

Merged

cmsbuild modified the milestones: CMSSW_14_1_X, CMSSW_14_2_X Aug 27, 2024

cmsbuild modified the milestones: CMSSW_14_1_X, CMSSW_14_2_X Sep 3, 2024

cmsbuild mentioned this pull request Oct 1, 2024

[DQM] Fix syntax in python scripts #46166

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Outer Tracker Phase2 DQM: additional stub validation plots #45475

Outer Tracker Phase2 DQM: additional stub validation plots #45475

brandiskip commented Jul 16, 2024

cmsbuild commented Jul 16, 2024 •

edited

Loading

cmsbuild commented Jul 16, 2024

cmsbuild commented Jul 16, 2024

tomalin commented Jul 18, 2024

sroychow Jul 17, 2024

sroychow Jul 17, 2024

sroychow Jul 17, 2024

tomalin Jul 18, 2024

cmsbuild commented Jul 22, 2024

tomalin Jul 22, 2024

brandiskip Jul 22, 2024

tomalin Jul 23, 2024

tomalin Jul 22, 2024

tomalin Jul 22, 2024

tomalin Jul 22, 2024

tomalin Jul 22, 2024

tomalin commented Jul 22, 2024

tomalin commented Jul 22, 2024

cmsbuild commented Aug 27, 2024

antoniovilela commented Sep 3, 2024

Outer Tracker Phase2 DQM: additional stub validation plots #45475

Are you sure you want to change the base?

Outer Tracker Phase2 DQM: additional stub validation plots #45475

Conversation

brandiskip commented Jul 16, 2024

PR description:

PR validation:

cmsbuild commented Jul 16, 2024 • edited Loading

cmsbuild commented Jul 16, 2024

cmsbuild commented Jul 16, 2024

tomalin commented Jul 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmsbuild commented Jul 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomalin commented Jul 22, 2024

tomalin commented Jul 22, 2024

cmsbuild commented Aug 27, 2024

antoniovilela commented Sep 3, 2024

cmsbuild commented Jul 16, 2024 •

edited

Loading