-
Notifications
You must be signed in to change notification settings - Fork 23
Added reduced-v3 ECal changes #1878
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: trunk
Are you sure you want to change the base?
Conversation
|
@SanjitMasanam FYI at this point it's certain that we wont have 3 double layers... The 2 double layers are likely, but then reading all of them out is still in the air, but I guess we can deal with that part upstream of the GDML |
Ok I'll edit the code to be only 2 double layers |
|
@tomeichlersmith @tvami, this PR is (approx.) ready. I was not able to go from 3 double layers to 2 because the GDML loops (see most relevant loop for this instance) don't like just having one iteration. The 3 double layers mean this loop iterates twice. If either of you know a workaround (other than making the loop stand-alone code which I considered but that would require rewriting many other loops as standalone which is more work than it's worth I think), it'd be great to have that workaround implemented. Otherwise, the current reduced_v3 has the first 2 double layers obviously so this subset would be accurate to the test run geometry. |
tomeichlersmith
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good first approximation at least from an Ecal-perspective.
|
Is it really hard to just move it out of the loop? |
A bit yeah but doable so it has been done. |
|
Thanks @SanjitMasanam ! Please edit the CI to use this, and also I think we can delete v2 |
DetDescr/python/EcalGeometry.py
Outdated
| eg = EcalGeometry(detectors_valid = ["ldmx-reduced-v3"], | ||
| gap = 1.5, | ||
| layerZPositions = [ | ||
| 7.582, 16.062, 32.226, 40.206, 55.37, 63.35 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked to Lincoln about the CAD model, there we have
86.00, 92.23, 136.8, 143.03 mm
I think we should edit that in the GDML too @SanjitMasanam
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, those distances are from the target? Also for editing that in the GDML, do you want me to add empty air of suitable thickness between each layer to achieve those distances?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no this is from the front of the ECAL box. We are about 26 inch away from the "target" (there is no target), but that is to be measured tomorrow (fingers crossed).
Sounds good about the air, yeah! Thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So 92.23-86=6.23 which is less than 16.062-7.582=8.48. How did that end up happening? I can add air for the other layers but I'll need some guidance on what to remove for the 2nd layer I think.
I am updating ldmx-sw, here are the details.
What are the issues that this addresses?
This resolves #1877
Check List
Important ECal changes for reduced-v3 vs. reduced-v2:
To test these changes, I ran two simulations (and a check):
v2 results:
v3 results:
Conclusion:
The results of both simulations (and the check) match expectation.