Skip to content
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

feat: import full HRPPD GDML as mRICH photo-detector #382

Closed
wants to merge 11 commits into from

Conversation

wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Mar 6, 2023

Briefly, what does this PR introduce?

Instead of implementing a detailed HRPPD model in xml with support in the geometry plugin, this just pulls in the GDML model from @alexander-kiselev.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators @osbornjd @msar15

Does this PR introduce breaking changes? What changes might users need to make to their code?

No.

Does this PR change default behavior?

No.

@github-actions github-actions bot added topic: backward Negative-rapidity detectors (electron-going side) topic: PID Particle identification labels Mar 6, 2023
@wdconinc
Copy link
Contributor Author

wdconinc commented Mar 7, 2023

Looks like this (only one box shown with children):
image

@wdconinc wdconinc requested a review from c-dilks March 7, 2023 01:29
@wdconinc
Copy link
Contributor Author

wdconinc commented Mar 7, 2023

@c-dilks Asked you to review this. Maybe there are useful approaches here for the dRICH? I think you should be able to get volumes by name out of the gdml, and then assign them sensor/module/segmentation. It is possibly a good 'CAD-light' approach.

@alexander-kiselev
Copy link

Perhaps. In a different context (and in a simple model case, where one can actually code them in OpenCascade directly) I'm using a different approach: have a CAD model and a gerber set for the matching PCB production done in the same ROOT script, in which case both models are automatically synchronized in terms of shapes, sizes, hole locations and such, as long as one does not make a coding mistake. It is obvious one can do the same for a CAD+GEANT output rather than a CAD+PCB combo. Essentially one is generating a GDML file and a CAD model of identical complexity at the same time. But then one can argue, why not use ROOT->CAD export, which is readily available. Anyway, my 2 cents.

@c-dilks
Copy link
Member

c-dilks commented Mar 8, 2023

Whatever we do, it should be sustainable and reproducible; it would be good to document where these gdml files come from and how they were produced (I will open an issue, this shouldn't block this PR).

Speaking of documentation, should these gdml files have the usual copyright headers?

@c-dilks
Copy link
Member

c-dilks commented Mar 8, 2023

Looks great, however there are some mRICH-mRICH overlaps, e.g.:

Overlap is detected for volume MRICH_module1_39:39 (G4Trd) with MRICH_module1_35:35 (G4Trd)
Overlap is detected for volume MRICH_module1_59:59 (G4Trd) with its mother volume MRICH_Envelope (G4Tubs)

https://github.com/eic/epic/actions/runs/4349674426/jobs/7599660505

@wdconinc wdconinc enabled auto-merge (squash) June 5, 2023 12:35
@alexander-kiselev
Copy link

alexander-kiselev commented Jun 25, 2023 via email

@wdconinc
Copy link
Contributor Author

This branch (obviously) needs work since we changed the default to pfrich. There is no real value in having the mrich implemented in more detail, but there is value in having a proof of concept of having an imported gdml component to point to as an example.

@wdconinc wdconinc disabled auto-merge June 25, 2023 02:18
@wdconinc
Copy link
Contributor Author

There are now several better examples of importing gdml in production code, so I'm going to close this.

@wdconinc wdconinc closed this Mar 14, 2024
@wdconinc wdconinc deleted the mrich-hrppd-as-gdml branch March 14, 2024 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: backward Negative-rapidity detectors (electron-going side) topic: PID Particle identification
Projects
Development

Successfully merging this pull request may close these issues.

3 participants