Skip to content

Conversation

@henrylay97
Copy link
Member

@henrylay97 henrylay97 commented Mar 21, 2025

Description

Move to GDML v02_04. The difference with respect to GDML v02_03 is the movement of all 7 CRT tagger walls with respect to the TPC to better represent what we see in data.

Commit ecb8643 shows the actual diff between 03 and 04.

I also add a pair of maps that provide the orientation & topend information for each module (the GDML purely simulates lumps of scintillator). These pieces of information were previously provided via if statements that have become more & more breakable as we have updated the GDML to reflect the more complex real life system. This prevents them from accidentally being broken by a GDML change.

Checklist

  • Added at least 1 label from available labels.
  • Assigned at least 1 reviewer under Reviewers,
  • Assigned all contributers including yourself under Assignees
  • Linked any relevant issues under Developement
  • Does this PR affect CAF data format? If so, please assign a CAF maintainer (PetrilloAtWork or JosiePaton) as additional reviewer.
  • Does this affect the standard workflow?

Relevant PR links (optional)

Another sbndcode PR (#694) contains related work on the matching algorithms but they can be treated independently from the point of view of release management despite resulting from the same work.

Link(s) to docdb describing changes (optional)

I am yet to present this work to the simulation group (although it has been presented to the CRT group). The simulation group will be shown on Tuesday (this is the live link to the slides I intend to show) but given the production schedule I wanted to get the PR logged.

@henrylay97 henrylay97 added crt Cosmic Ray Tagger geometry Geometry related issue labels Mar 21, 2025
@henrylay97 henrylay97 self-assigned this Mar 21, 2025
Copy link
Member

@marcodeltutto marcodeltutto left a comment

Choose a reason for hiding this comment

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

Thanks, @henrylay97! Look great.

Only one thing: something that we have forgotten to do in the past is that, every time the gdml of flux changes, we need to update a "geomscan" file, which is needed for GENIE rock box. I made a PR (still pending, #691) which updates this file to gdml 02_03.

I also just generated a new file for your new gdml 02_04, and asked @bear-is-asleep to upload it to sbnd_data. Can I ask you to update the file name to point to this new file? This needs to be done here:
https://github.com/SBNSoftware/sbndcode/blob/feature/hlay_gdml_v02_04/sbndcode/LArSoftConfigurations/gen/genie_sbnd.fcl#L379
The new name is sbnd_rock_maxpathlength_fluxL_gdmlv02_04.xml.

@henrylay97
Copy link
Member Author

Okay - thanks for doing the hard bit for me Marco!

PR is now updated. @bear-is-asleep we'll need to resolve the obvious easy conflict when #691 has been merged (before this PR).

Copy link

@afm1g15 afm1g15 left a comment

Choose a reason for hiding this comment

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

Looks good to me @henrylay97! I built it with PR #694 and had a look, all seems fine to me

@afm1g15 afm1g15 mentioned this pull request Mar 25, 2025
6 tasks
@bear-is-asleep
Copy link
Contributor

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_06 SBNSoftware/sbncode@v10_04_06_01

@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build SBND phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link
Collaborator

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

@bear-is-asleep bear-is-asleep moved this from Todo to To be Merged (RM only) in SBND March 2025 production Mar 27, 2025
@bear-is-asleep
Copy link
Contributor

trigger build LArSoft/lar*@LARSOFT_SUITE_v10_04_06 SBNSoftware/sbncode@v10_04_06_p01

@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for e26:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

✔️ CI build for LArSoft Succeeded on slf7 for c14:prof -- details available through the CI dashboard

@FNALbuild
Copy link
Collaborator

❌ CI build for SBND Failed at phase build SBND on slf7 for c14:prof -- details available through the CI dashboard

🚨 For more details about the failed phase, check the build SBND phase logs

parent CI build details are available through the CI dashboard

@FNALbuild
Copy link
Collaborator

⚠️ CI build for SBND Warning at phase ci_tests SBND on slf7 for e26:prof -- details available through the CI dashboard

🚨 For more details about the warning phase, check the ci_tests SBND phase logs

parent CI build details are available through the CI dashboard

@henrylay97
Copy link
Member Author

Other than the usual changes like the detsim variations this looks good to me, changes are fine!

@bear-is-asleep
Copy link
Contributor

Approved

@bear-is-asleep bear-is-asleep moved this from To be Merged (RM only) to Approved (RM only) in SBND March 2025 production Mar 28, 2025
@henrylay97
Copy link
Member Author

Hi @jzennamo @lyates17 @marcodeltutto.

Following discussion at last week's simulation meeting (sorry it took a while), I have rereferenced the CRT positions so that they mainly depend on the TPC position not the cryostat position.

  • This does not change GDML v02_04 at all. This was tested by running the "preparsing" of the base file to create the two 'actual' GDML files which do not change at all!
  • There are a couple of references to the cryostat width that are retained but no references to it's position.
  • Conflicts could arise in a future situation if the TPC was moved within the cryostat such that a CRT wall was pulled into an 'overlap' with another object (floor, wall, cryostat etc) but that is unlikely and would create angry messages from geant4.

Would be great if at least one of you could re-approve this PR :D

Copy link
Member

@marcodeltutto marcodeltutto left a comment

Choose a reason for hiding this comment

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

Thanks so much, @henrylay97!

Just tested it and indeed no changes!

@henrylay97
Copy link
Member Author

Thanks for testing Marco! :D

@bear-is-asleep
Copy link
Contributor

@henrylay97 once the refs are updated this will need to be retested with the CI

@bear-is-asleep bear-is-asleep moved this from Approved (RM only) to To be Merged (RM only) in SBND March 2025 production Apr 1, 2025
@bear-is-asleep bear-is-asleep merged commit d0a3597 into develop Apr 1, 2025
@bear-is-asleep bear-is-asleep moved this from To be Merged (RM only) to In tagged release in SBND March 2025 production Apr 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crt Cosmic Ray Tagger geometry Geometry related issue

Projects

Status: In tagged release

Development

Successfully merging this pull request may close these issues.

6 participants