Including Metrology Mask Rotation by ckierans · Pull Request #74 · cositools/nuclearizer

ckierans · 2025-11-25T02:59:25Z

I've added the ability to include the Yaw value from the mask metrology measurements to account for rotations of the strips on the LV and HV side.

If the MaskMetrology is enabled in the Depth Calibration module, then the position of the hit is determined by the intercept of the dominate (i.e. largest energy deposit) LV and HV strips, where the centroid of the strip and angle are read from the metrology file. The intercept is calculated in a new function GetStripIntersection, and the metrology file is loaded with a new function LoadMaskMetrologyFile.

TODO: Aldo's file has the HV strip ID flipped (as evident in the x,y position reported in the metrology files), so there is a int HVStripID_flipped = 63 - HVStripID; placeholder until the files are fixed.

TODO: The initial X/Y position should be more rigorous to include charge sharing down the line. This method will need to be updated at that time.

I confirmed that the implementation in nuclearizer gives ARM values that are the same as without the correction (unfortunately, not actually measurably better).

…till needs work

…works for 2 detectors based on latest files from Aldo

* Merging select files from StripPairingTesting into StripPairingCleanUp so I can do pull request * Clean up comments. Remove long debugging statements for readability * Calling charge trapping correction function in chi square calculation and hit population * Cleaning up comments * Reformatting strip pairing module * Update copywrite. Change name of module. * Fixing documentation in header files * Change how strip pairing chi square is read out to .dat file * Adding comments for all the nested vectors * Adding new version to StreamDat that reads out LV and HV energy of each hit * Adding Sean's new strip pairing expos to chi square version of strip pairing * Changing all instances of RedChiSquare to ReducedChiSquare for clarity * Flags for multiple hits on a single strip are now defined on the MHit level * Initializing m_StripHitMultipleTimesX and m_StripHitMultipleTimesY to false * Changing StreamDAT version from Version 1 to Version 3 * Rebased and fixed merge conflict with strip pairing --------- Co-authored-by: Julian Gerber <[email protected]>

ckierans · 2025-12-09T03:43:59Z

This is ready to review, with the acknowledgement that the X/Y position will needs to be updated based on the coordinate system we define. This can be done in a future small PR.

zoglauer · 2025-12-09T16:52:13Z

include/MGUIOptionsDepthCalibration2024.h

+  //! Select mask metrology file to load. This gives the translation and rotation for each strip in the detector frame
+  MGUIEFileSelector* m_MaskMetrologyFileSelector;
+
+  int m_UseMaskMetCorr;


zoglauer · 2025-12-09T16:52:28Z

include/MModuleDepthCalibration2024.h

@@ -1,4 +1,4 @@
-/*
+/*:q


zoglauer · 2025-12-09T16:53:51Z

src/MGUIOptionsDepthCalibration2024.cxx

+//  TGLayoutHints* FileLabelLayout = new TGLayoutHints(kLHintsTop | kLHintsExpandX, m_FontScaler*65 + 21*m_FontScaler, m_FontScaler*65, 0, 2*m_FontScaler);
+
+  m_MaskMetrologyFileSelector = new MGUIEFileSelector(m_OptionsFrame, "", dynamic_cast<MModuleDepthCalibration2024*>(m_Module)->GetMaskMetrologyFileName());
+  m_MaskMetrologyFileSelector->SetFileType("metrology", "*.csv");


We have so many css files, can we name this *.metrology.csv or soemthing similar?

zoglauer · 2025-12-09T16:54:07Z

src/MGUIOptionsDepthCalibration2024.cxx

+      switch (Parameter1) {
+        case c_MetFile:
+          if (m_MaskMetModeCB->GetState() == kButtonDown) {
+            m_UseMaskMetCorr = 1;


zoglauer · 2025-12-09T16:54:43Z

src/MModuleDepthCalibration2024.cxx

-        double Xpos = m_YPitches[DetID]*((m_NYStrips[DetID]/2.0) - ((double)LVStripID));
-        double Ypos = m_XPitches[DetID]*((m_NXStrips[DetID]/2.0) - ((double)HVStripID));
-        double Zpos = 0.0;
+	double Xpos, Ypos, Zpos;


Please initialize all variables in C/C++

zoglauer · 2025-12-09T16:55:08Z

src/MModuleDepthCalibration2024.cxx

-        double Zpos = 0.0;
+	double Xpos, Ypos, Zpos;
+
+        if ( m_MaskMetrologyEnabled ) {


Formatting: if (sdkjvghjk == true) {

zoglauer · 2025-12-09T16:56:00Z

src/MModuleDepthCalibration2024.cxx

 std::vector<double>* MModuleDepthCalibration2024::GetPixelCoeffs(int PixelCode)
 {
  // Check to see if the stretch and offset have been loaded. If so, try to get the coefficients for the specified pixel.
  if( m_CoeffsFileIsLoaded ){


zoglauer · 2025-12-09T16:56:39Z

src/MModuleDepthCalibration2024.cxx

+  //Read the Mask Metrology File
+  // Det ID, Side (l,h), Strip ID (0-63), x_mm, y_mm, z_mm, roll_deg, pitch_deg, yaw_deg
+  MFile F;
+  if( F.Open(FName) == false ){


Formatting... There are a bunch of these... I no longer mention them but please fix

zoglauer · 2025-12-09T16:57:30Z

src/MModuleDepthCalibration2024.cxx

+  } else {
+    MString Line;
+    while( F.ReadLine( Line ) ){
+      if ( Line.BeginsWith('#') ){        


if (Line.BeginsWith('#') == true) continue;
This makes it easier to read

zoglauer · 2025-12-09T16:58:28Z

src/MModuleDepthCalibration2024.cxx

+      }
+      else {
+        std::vector<MString> Tokens = Line.Tokenize(",");
+        if( Tokens.size() == 9 ){


Dl we want an error message if it is not 9?

zoglauer · 2025-12-09T16:59:24Z

src/MModuleDepthCalibration2024.cxx

  }

+  MXmlNode* MasKMetrologyNode = Node->GetNode("MaskMetrology");
+  if( MasKMetrologyNode != NULL ){


!= nullptr
NULL is just for C

zoglauer · 2025-12-09T17:00:44Z

src/MModuleDepthCalibration2024.cxx


+  MXmlNode* MasKMetrologyNode = Node->GetNode("MaskMetrology");
+  if( MasKMetrologyNode != NULL ){
+      m_MaskMetrologyEnabled = (bool) MasKMetrologyNode->GetValueAsInt();


GetValueAsBoolean()

zoglauer · 2025-12-09T17:00:59Z

src/MModuleDepthCalibration2024.cxx

+  }
+
+  MXmlNode* MaskMetrologyFileNameNode = Node->GetNode("MaskMetrologyFileName");
+  if (MaskMetrologyFileNameNode != 0) {


zoglauer · 2025-12-09T17:01:17Z

src/MModuleDepthCalibration2024.cxx

+  }
+
  MXmlNode* UCSDOverrideNode = Node->GetNode("UCSDOverride");
  if( UCSDOverrideNode != NULL ){


zoglauer · 2025-12-09T17:01:49Z

src/MModuleDepthCalibration2024.cxx

+
  MXmlNode* UCSDOverrideNode = Node->GetNode("UCSDOverride");
  if( UCSDOverrideNode != NULL ){
      m_UCSDOverride = (bool) UCSDOverrideNode->GetValueAsInt();


GetValueAsBoolean

zoglauer · 2025-12-09T17:02:15Z

src/MModuleDepthCalibration2024.cxx

  MXmlNode* Node = new MXmlNode(0,m_XmlTag);
  new MXmlNode(Node, "CoeffsFileName", m_CoeffsFile);
  new MXmlNode(Node, "SplinesFileName", m_SplinesFile);
+  new MXmlNode(Node, "MaskMetrology", (unsigned int) m_MaskMetrologyEnabled);


bool should work

zoglauer · 2025-12-09T17:02:33Z

src/MModuleDepthCalibration2024.cxx

  new MXmlNode(Node, "SplinesFileName", m_SplinesFile);
+  new MXmlNode(Node, "MaskMetrology", (unsigned int) m_MaskMetrologyEnabled);
+  new MXmlNode(Node, "MaskMetrologyFileName", m_MaskMetrologyFile);
  new MXmlNode(Node, "UCSDOverride",(unsigned int) m_UCSDOverride);  


bool should work

include/MModuleDepthCalibration2024.h

…works for 2 detectors based on latest files from Aldo

* Merging select files from StripPairingTesting into StripPairingCleanUp so I can do pull request * Clean up comments. Remove long debugging statements for readability * Calling charge trapping correction function in chi square calculation and hit population * Cleaning up comments * Reformatting strip pairing module * Update copywrite. Change name of module. * Fixing documentation in header files * Change how strip pairing chi square is read out to .dat file * Adding comments for all the nested vectors * Adding new version to StreamDat that reads out LV and HV energy of each hit * Adding Sean's new strip pairing expos to chi square version of strip pairing * Changing all instances of RedChiSquare to ReducedChiSquare for clarity * Flags for multiple hits on a single strip are now defined on the MHit level * Initializing m_StripHitMultipleTimesX and m_StripHitMultipleTimesY to false * Changing StreamDAT version from Version 1 to Version 3 * Rebased and fixed merge conflict with strip pairing --------- Co-authored-by: Julian Gerber <[email protected]>

* Set the geometry in SMEX DEE SubModules * Read detector dimensions from the geometry * Read detector radius from geometry

Merging in formating fixes after Andreas's review, rebased with develop/em

ckierans · 2025-12-10T05:12:36Z

Sorry the commit history turned so messy. I had to resolve the merge conflict with Sean's PR that I merged this morning and obviously didn't do it in the cleanest way.

But all of your comments should be address. I cleaned up Sean's code a bit too.

ckierans · 2026-01-21T02:37:05Z

@zoglauer do you mind checking my changes here? I'd like to merge this PR since I cleaned up Sean's Depth Calibration code pretty substantially, but want to keep making progress on that module in a separate PR.

I'm still waiting on the updated metrology file from Aldo to get this finalized, but the functionality should be right.

zoglauer · 2026-01-21T07:39:20Z

include/MGUIOptionsDepthCalibration2024.h

+  //! Select mask metrology file to load. This gives the translation and rotation for each strip in the detector frame
+  MGUIEFileSelector* m_MaskMetrologyFileSelector;
+
+  bool m_UseMaskMetCorr;


Please describe everything

zoglauer · 2026-01-21T07:41:38Z

src/MModuleDepthCalibration2024.cxx

  }

+  if (m_MaskMetrologyEnabled == true) {
+    cout << "\n  !!! Mask Metrology Enabled !!! \n" << endl;


Standard format please:
if (g_Verbosity >= c_Info) cout<<m_XmlTag<<": ..."<<endl;
And please use endl instead of \n

zoglauer · 2026-01-21T07:47:00Z

src/MModuleDepthCalibration2024.cxx

+  // Det ID, Side (l,h), Strip ID (0-63), x_mm, y_mm, z_mm, roll_deg, pitch_deg, yaw_deg
+  MFile MetrologyFile;
+  if (MetrologyFile.Open(FileName) == false) {
+    cout << "ERROR in MModuleDepthCalibration2024::LoadMaskMetrologyFile: failed to open metrology file." << endl;


Standard format please:
if (g_Verbosity >= c_Error) cout<<m_XmlTag<<": ..."<<endl;

zoglauer · 2026-01-21T07:48:37Z

src/MModuleDepthCalibration2024.cxx

+  vector<double> LVStripMet = m_MaskMetrology[R_LVStrip];
+  vector<double> HVStripMet = m_MaskMetrology[R_HVStrip];
+
+  //Find the x position of two lines represented by the dominate strips:


White space:
// Find
// LVstrip

zoglauer · 2026-01-21T07:54:39Z

src/MModuleDepthCalibration2024.cxx

+  // HVstrip is centered at (x,y,z) = (hv_strip_met[0], hv_strip_met[1], hv_strip_met[2])
+  // and is approximately parallel to the x axis, but rotated at angle (hv_strip_met[5] - pi/2) 
+  // around the z axis of the detector
+  double XIntercept = (HVStripMet[0]*tan((HVStripMet[5]-90)*TMath::DegToRad()) - LVStripMet[0]/tan(LVStripMet[5]*TMath::DegToRad()) - LVStripMet[1] + HVStripMet[1])/(tan((HVStripMet[5]-90)*TMath::DegToRad())-1/tan(LVStripMet[5]*TMath::DegToRad()));


Are you 100% sure there is never ever any possibility of a division by zero, or any other issues?

Even if you are 99.999% sure, check for it...

Merged latest develop/em into metrology fix before updating PR

Updating remote origin/metrology_fix branch with latest PRs

ckierans and others added 4 commits November 24, 2025 21:19

ADD: Including Mask Metrology file in Depth Calibration, correction s…

6d926ee

…till needs work

CHG: Updated x,y definition with mask rotation, added print statements

1f2163f

Updated metrology with correct understanding of rotation angles, now …

5577c2e

…works for 2 detectors based on latest files from Aldo

ckierans requested a review from zoglauer December 9, 2025 03:43

ckierans linked an issue Dec 9, 2025 that may be closed by this pull request

Mask Metrology corrections #51

Open

zoglauer reviewed Dec 9, 2025

View reviewed changes

include/MModuleDepthCalibration2024.h Outdated

@@ -1,4 +1,4 @@

/*

/*:q

Copy link

Collaborator

zoglauer Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove :q

zoglauer reviewed Dec 9, 2025

View reviewed changes

include/MModuleDepthCalibration2024.h Show resolved Hide resolved

ckierans and others added 5 commits December 9, 2025 22:59

Resolving merge conflict with Sean's recent PR

6ba6e6b

CHG: Updated x,y definition with mask rotation, added print statements

beb5c32

Updated metrology with correct understanding of rotation angles, now …

c138b17

…works for 2 detectors based on latest files from Aldo

Resolving merge conflict with Sean's recent PR

f858369

ckierans and others added 14 commits December 9, 2025 23:19

Resolving merge conflict with Sean's recent PR

108275f

[DEE] Improve strip ID handling for guard ring events

e0861b2

Add TODOs

7c3d1d6

ADD: How to handle ToDo's

3d3e0e3

CHG: Updated README.md

3eeacff

CHG: Add historic contributors

5038c55

Resolving merge conflict with Sean's recent PR

2fb1580

Cleanup: added comments and cleaned up extraneous print statements

230f190

BUG: Clear variables on Finalize

649f74c

[DEE] Read detector dimensions from the geometry (cositools#76)

f5c13f0

* Set the geometry in SMEX DEE SubModules * Read detector dimensions from the geometry * Read detector radius from geometry

Resolving merge conflict with Sean's recent PR

22f3214

CHG: Updated x,y definition with mask rotation, added print statements

a5a5de2

Merge branch 'metrology' into metrology_merge

1d90cc8

Merging in formating fixes after Andreas's review, rebased with develop/em

Removing a vim mistype

92ed5b7

Merge branch 'develop/em' into metrology_fix

3e6d188

zoglauer reviewed Jan 21, 2026

View reviewed changes

ckierans added 5 commits February 3, 2026 13:01

Checked for division by zero

573f895

Added better comments, and fixed zero division bug

7a20d99

Merge remote-tracking branch 'upstream/develop/em' into metrology_merge

ab024e5

Merged latest develop/em into metrology fix before updating PR

Merge remote-tracking branch 'origin/metrology_fix' into metrology_merge

1912f7b

Updating remote origin/metrology_fix branch with latest PRs

Updated instances of cout with standard verbosity format

59a25a3

Conversation

ckierans commented Nov 25, 2025

Uh oh!

ckierans commented Dec 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ckierans commented Dec 10, 2025

Uh oh!

ckierans commented Jan 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoglauer Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zoglauer Jan 21, 2026 •

edited

Loading