Including Metrology Mask Rotation #74
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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 <jmgerber@ucsd.edu>
Collaborator
Author
|
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
reviewed
Dec 9, 2025
| //! 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
reviewed
Dec 9, 2025
| @@ -1,4 +1,4 @@ | |||
| /* | |||
| /*:q | |||
zoglauer
reviewed
Dec 9, 2025
| // 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"); |
Collaborator
There was a problem hiding this comment.
We have so many css files, can we name this *.metrology.csv or soemthing similar?
zoglauer
reviewed
Dec 9, 2025
| switch (Parameter1) { | ||
| case c_MetFile: | ||
| if (m_MaskMetModeCB->GetState() == kButtonDown) { | ||
| m_UseMaskMetCorr = 1; |
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| 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; |
Collaborator
There was a problem hiding this comment.
Please initialize all variables in C/C++
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| double Zpos = 0.0; | ||
| double Xpos, Ypos, Zpos; | ||
|
|
||
| if ( m_MaskMetrologyEnabled ) { |
Collaborator
There was a problem hiding this comment.
Formatting: if (sdkjvghjk == true) {
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| 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
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| //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 ){ |
Collaborator
There was a problem hiding this comment.
Formatting... There are a bunch of these... I no longer mention them but please fix
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| } else { | ||
| MString Line; | ||
| while( F.ReadLine( Line ) ){ | ||
| if ( Line.BeginsWith('#') ){ |
Collaborator
There was a problem hiding this comment.
if (Line.BeginsWith('#') == true) continue;
This makes it easier to read
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| } | ||
| else { | ||
| std::vector<MString> Tokens = Line.Tokenize(","); | ||
| if( Tokens.size() == 9 ){ |
Collaborator
There was a problem hiding this comment.
Dl we want an error message if it is not 9?
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| } | ||
|
|
||
| MXmlNode* MasKMetrologyNode = Node->GetNode("MaskMetrology"); | ||
| if( MasKMetrologyNode != NULL ){ |
Collaborator
There was a problem hiding this comment.
!= nullptr
NULL is just for C
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
|
|
||
| MXmlNode* MasKMetrologyNode = Node->GetNode("MaskMetrology"); | ||
| if( MasKMetrologyNode != NULL ){ | ||
| m_MaskMetrologyEnabled = (bool) MasKMetrologyNode->GetValueAsInt(); |
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| } | ||
|
|
||
| MXmlNode* MaskMetrologyFileNameNode = Node->GetNode("MaskMetrologyFileName"); | ||
| if (MaskMetrologyFileNameNode != 0) { |
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| } | ||
|
|
||
| MXmlNode* UCSDOverrideNode = Node->GetNode("UCSDOverride"); | ||
| if( UCSDOverrideNode != NULL ){ |
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
|
|
||
| MXmlNode* UCSDOverrideNode = Node->GetNode("UCSDOverride"); | ||
| if( UCSDOverrideNode != NULL ){ | ||
| m_UCSDOverride = (bool) UCSDOverrideNode->GetValueAsInt(); |
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| 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); |
zoglauer
reviewed
Dec 9, 2025
src/MModuleDepthCalibration2024.cxx
Outdated
| 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); |
zoglauer
reviewed
Dec 9, 2025
| map<MReadOutElementDoubleStrip, vector<double>> m_MaskMetrology; | ||
|
|
||
|
|
||
| // boolean for use with the card cage at UCSD since it tags all events as detector 11 |
Collaborator
There was a problem hiding this comment.
Do we still need that?
The UCSD code should be its own branch and not be in the main code?
Maybe make an issue
Collaborator
Author
There was a problem hiding this comment.
I created an issue for this. Leaving this code for now.
…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 <jmgerber@ucsd.edu>
* 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
Collaborator
Author
|
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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).