Skip to content

dRICH tiled aerogel; First attempt #840

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

dRICH tiled aerogel; First attempt #840

wants to merge 7 commits into from
+80 −11

Conversation

chchatte92
Copy link
Member

@chchatte92 chchatte92 commented Mar 12, 2025 •
edited
Loading

Briefly, what does this PR introduce?

The dRICH aerogel so far has been a disk, which is unrealistic. We are tiling the aerogel, and adding carbon fiber ribs. This has consequences in terms of detector acceptance and efficiency. This PR makes a very preliminary version to tile the aerogel as a cone segment. The right shape and dimension has not yet been finalized. This draft PR aims to work with collaborators (Luisa and Rohit), for performing further test, improving coding styles and remove the hardcoded parameters and make the nesting as much as possible user-tunable.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • [] New feature (issue #__)
  • Documentation update
  • [X ] Other:
    Before dRICH aerogel was a single disk, now we are tiling the aerogel (with realistic shapes and dimensions) .

Please check if this PR fulfills the following:

  • [X ] Tests for the changes have been added
  • Some preliminary tests to remove WARN and bugs has been made. Working up to the benchmark level.
  • Documentation has been added / updated
  • [ X] Changes have been communicated to collaborators
  • Luisa Occhiutto and Rohit Singh has been communicated and they will be working on this branch for the finalization.
    At this moment this is not a final version. The right aerogel tile shape and choices are yet to be finalized and acceptanceXefficiency studies are to be performed. It will remain as a draft PR for the time being. Once the right shape and dimensions are finalized, we will create a final PR.
  • It is also observed that neither Luisa nor Rohit is in the dev group. They have been communicated to request for a permission.

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

No

Does this PR change default behavior?

No

@chchatte92 chchatte92 requested review from wdconinc and c-dilks March 12, 2025 11:53
@chchatte92 chchatte92 self-assigned this Mar 12, 2025
@github-actions github-actions bot added topic: forward Positive-rapidity detectors (hadron-going side) topic: PID Particle identification labels Mar 12, 2025
@chchatte92
Copy link
Member Author

Before the aerogel was placed like this:
image

Now it looks like this:
image

(I am also curious that why the geometry is now monochromatic)

@chchatte92
Copy link
Member Author

There are a few different possibilities to tile the aerogel (considering the material loss at construction level) and to minimize aerogel material production.
One can obviously go as the aerogel tiling concept in Belle-2 ARICH.
We will test these different possibilities (in separate branches of ePIC, if possible) and to create a final PR for the specific branch which seems more effective in terms of acceptanceXefficiency and production feasibility.
The choices has been discussed in the dRICH general meeting dedicated to mechanics:
https://indico.bnl.gov/event/26331/contributions/103316/attachments/60053/103148/EPIC_dRICH_19022025.pdf

@chchatte92 chchatte92 changed the title dRich tiled aerogel dRICH tiled aerogel; First attempt Mar 12, 2025
@chchatte92 chchatte92 force-pushed the dRICH_tiled_aerogel branch from 88ab284 to f1f7673 Compare March 12, 2025 14:04
@c-dilks c-dilks linked an issue Mar 12, 2025 that may be closed by this pull request
dRICH/pfRICH: tile the aerogel #166
Open
@c-dilks
Copy link
Member

c-dilks commented Mar 12, 2025

Looks good, I linked this PR to close #166.

I'll leave the code review to @wdconinc.

One question: will this need any change in EICrecon's richgeo service, i.e., the stuff that links this geometry to ACTS surfaces and IRT geometry objects? I assume it shouldn't, but it would be wise to test the IRT PID implementation (if you still use it).

@chchatte92
Copy link
Member Author

chchatte92 commented Mar 12, 2025 •
edited
Loading

Looks good, I linked this PR to close #166.

I'll leave the code review to @wdconinc.

One question: will this need any change in EICrecon's richgeo service, i.e., the stuff that links this geometry to ACTS surfaces and IRT geometry objects? I assume it shouldn't, but it would be wise to test the IRT PID implementation (if you still use it).

Hi @c-dilks, I tested the whole chain, and it goes through without complains. I guess because, still we have one layer of 4 cm aerogel.
I get a plot like this:
image
The plot seems reasonable to me. As you can see systematically for each bin for lower number of photons, the tiled aerogel histograms have higher entries and for the larger photon counts the single disk configuration has higher counts.
This is what I would intuitively assume. The carbon fibre ribs are eating some photons. But, we will definitely test the features.

@wdconinc wdconinc requested a review from veprbl March 12, 2025 17:56
@wdconinc
Copy link
Contributor

I'm going to let @veprbl @Chao1009 @rahmans1 review this. I'm trying to focus on operations aspects over development at this point.

@wdconinc
Copy link
Contributor

(but you might get more reviews when the overlap checks succeed)

@wdconinc wdconinc removed their request for review March 13, 2025 00:49
veprbl
veprbl reviewed Mar 13, 2025
View reviewed changes
Copy link
Member

@veprbl veprbl left a comment

Choose a reason for hiding this comment

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

Please fix overlap checks. You can test it locally using checkOverlaps -c ${DETECTOR_PATH}/epic_craterlake.xml and/or scripts/checkOverlaps.py -c ${DETECTOR_PATH}/epic_craterlake.xml.

src/DRICH_geo.cpp
// aerogel placement and surface properties
// TODO [low-priority]: define skin properties for aerogel and filter
// FIXME: radiatorPitch might not be working correctly (not yet used)
/*
Copy link
Member

Choose a reason for hiding this comment

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

No commented code is to be stored in the public repo, please just remove the block.

src/DRICH_geo.cpp
@@ -318,26 +379,29 @@ static Ref_t createDetector(Detector& desc, xml::Handle_t handle, SensitiveDetec
radiatorRmax +
snoutDelta * (aerogelThickness + airgapThickness + filterThickness) / snoutLength);

Volume aerogelVol(detName + "_aerogel", aerogelSolid, aerogelMat);
//Volume aerogelVol(detName + "_aerogel", aerogelSolid, aerogelMat);
Copy link
Member

Choose a reason for hiding this comment

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

Same here and elsewhere.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @veprbl Dimtry, yes I know. I wanted to put this draft PR in order to make sure our young collaborators, work directly on a branch that is under radar of the software experts. As it will be part of their PhD studies. But, my impression is, they haven't yet managed to get an dev-access.
Hence I think I will take care of the overlap and other issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@veprbl veprbl

@c-dilks c-dilks

At least 1 approving review is required to merge this pull request.

Assignees

@chchatte92 chchatte92

Labels
topic: forward Positive-rapidity detectors (hadron-going side) topic: PID Particle identification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

dRICH/pfRICH: tile the aerogel
4 participants
@chchatte92 @c-dilks @wdconinc @veprbl