Skip to content

Proper numbering and orientations of PMTs in mPMT for WCTE#375

Merged
tdealtry merged 2 commits intoWCSim:developfrom
WCTE:WCTE_mPMT
Sep 18, 2023
Merged

Proper numbering and orientations of PMTs in mPMT for WCTE#375
tdealtry merged 2 commits intoWCSim:developfrom
WCTE:WCTE_mPMT

Conversation

@kmtsui
Copy link
Contributor

@kmtsui kmtsui commented Sep 11, 2023

To get the numbering and orientations correct for WCTE mPMTs. May need to further change the module dimensions later.
image

@tdealtry
Copy link
Contributor

Thanks @kmtsui This is modifying all mPMT geometries not just WCTE. Can you explain a bit more as to what changed and why?

I would presume the difference for all geometries is in WCSimMultiPMTParameterisation.cc

@kmtsui
Copy link
Contributor Author

kmtsui commented Sep 15, 2023

To change the ordering, a new mPMTconfig is used. But then there is an extra offset in the ring because copy number is directly used. That why I subtracted the number of PMTs from the previous rings.

Looking at the output, the difference in geometry positions is flipping between -0.000 and 0.000, so it should be numerical precision in the subtraction. Let me change the way of subtraction.

@kmtsui
Copy link
Contributor Author

kmtsui commented Sep 15, 2023

okay, the difference is still there for some geometries, probably because in the original code, phi is sometimes added over 2pi.

@tdealtry
Copy link
Contributor

Ok I see. Happy for you to merge it when you're happy with it, since the changes are small (< 1% level)

I just draw your attention to this showing that the WCTE detector size has been modified by this PR. Is this expected?
(the FD/IWCD/SK geometries, thankfully, haven't changed size)

@kmtsui
Copy link
Contributor Author

kmtsui commented Sep 18, 2023

Looking at WCSimConstructGeometryTables.cc, the detector size variables are calculated directly from the PMT positions instead of the mPMT module. Since the PMT numbering/angle are changed, these size variables are changed accordingly. Personally I prefer a better way to define the detector radius and height, maybe directly from WCIDDiameter and WCIDHeight?

Anyway I think we are good to merge it.

@tdealtry tdealtry merged commit c1cbc15 into WCSim:develop Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants