Skip to content

New far forward beampipe and cryostat model v2025 implementation #893

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

Open
wants to merge 85 commits into
base: main
Choose a base branch
from

Conversation

nat93
Copy link
Contributor

@nat93 nat93 commented Jun 19, 2025

Briefly, what does this PR introduce?

This PR introduces a complete model of the cryostat (fwd and bwd) with detailed description of superconducting coils, support tubes, yokes, yoke shielding, heat shielding, and cryostat vessel. The fwd and rear sides were adjusted in order to avoid overlaps with off-momentum detectors and low-Q^2 tagger pipes.

The corresponding STEP file can be found on SharePoint: EIC Public Sharing Docs → Documents → Experimental Program → ePIC → Engineering → STR-Files → IR6_CRYOSTAT_2200m_top_4-24-2025.stp [https://brookhavenlab.sharepoint.com/:u:/s/EICPublicSharingDocs/EdR44ODny1BEmMTTks61CCwBe8tJYMc0iWkPIYY_EjYEuw?e=2XLUEt]

The changes have been communicated to TIC, IR, and MDP communities.

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

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

Does this PR change default behavior?

Yes, it adds extra materials between the beam pipe (beam loss hot-spots) and the ePIC detector. It is crucial for the realistic detector background and dose calculations.

Andrii Natochii added 30 commits June 5, 2025 13:20
…de from the end of the center beam pipe to the end of the fwd cryostat <-- will be replaced by the realistic beam pipe model later
…P beam pipe and B0pF: covering in phi 280deg, instead of 260 deg to avoid overlaps with B0 cryostat and heatshield
@wdconinc
Copy link
Contributor

I will of course ensure that the necessary changes for the OMD and RP are in place before a new campaign would be launched with the changes

Considering how much we keep having to delay campaigns for late changes and requests, I would suggest that this work of implementing any changes to the OMD reconstruction matrices should start soon. It seems unlikely that we would merge this and knowingly break (apparently) also the far forward without any PR already in place.

@ajentsch
Copy link
Contributor

I will of course ensure that the necessary changes for the OMD and RP are in place before a new campaign would be launched with the changes

Considering how much we keep having to delay campaigns for late changes and requests, I would suggest that this work of implementing any changes to the OMD reconstruction matrices should start soon. It seems unlikely that we would merge this and knowingly break (apparently) also the far forward without any PR already in place.

I will do it this week - it's on my very long list.

@nat93
Copy link
Contributor Author

nat93 commented Jun 30, 2025

Hi @simonge,

I'm still not entirety comfortable with this PR. There is too much to review with due attention to ensure it is both appropriately maintainable and functional without dedicating a significant uninterrupted period of time. It would take less time to review and merge a series of incremental PRs. This might just be a flaw in my current attention span capacity.

There is only one .cpp file with two functions and four XML files describing the cryostat geometry. Nothing has changed for any of the systems except for the OMD, for which @ajentsch has provided a comprehensive explanation.
I believe it would be wise to focus our efforts on ensuring the geometry is updated, rather than on optimizing the design or performance - especially given that the current setup in DD4Hep is outdated and will never be built. Furthermore, we are all aware that additional updates are on the way, which will inevitably affect the detectors: a new beam pipe from the vacuum group, new magnets, and more.
We are already behind with the B2eR update, which, as decided by the Project, must be moved outside the cryostat. Of course, we can propose further updates to the warm B2eR, but only after implementing the changes that have already been decided.

Furthermore, radiation calculations are an ultra-high priority. They are essential for machine and detector protection, including the design and placement of collimators, shielding, optics tuning, and the vacuum system. From my perspective, this currently appears to be the most pressing/urgent issue (<-- rephrased to avoid misunderstanding). We must implement the geometry and run full radiation simulations - including TID, background rates, and related metrics - to determine whether ePIC can withstand the expected conditions.

Why have you moved the far_backward magnet definitions out of magnets and into beamline_extension_electrons?

I did this for consistency. The HSR BWD/FWD magnets are defined in beamline_extension_hadron.xml/ion_beamline.xml, while the ESR BWD/FWD magnets are defined in beamline_extension_electron.xml/electron_beamline.xml. This separation makes it clear where to look for each ring.

This cpp file is used only for cryostat magnets, so there is no need to create a separate - it is already a separate class. For most of the elements, the common coordinate is the yoke position. However, there are many individual components that are not aligned with the yoke, such as holes, electron beam pipe elements and magnets.
This will likely highlight my ignorance of the magnet geometry. The Q1eR and Q2eR coils and support tubes are already factorized out from the cryostat volume with the cryostat volume built around them from the Q1APR etc definition. Could we not do the same for the Q1APR etc. making the xml files and geometry hierarchy easier to understand?

In the latest commit, I have moved BWD cryo magnets from far_backward/beamline_extension_e/h.xml to far_backward/magnets.xml.

Please remove the white space between the numbers, units and operators in the xml.

I have removed the white space between the numbers, units and operators in the xml.

Please update the copyright statements to include your name.

I have added my name.

@wdconinc
Copy link
Contributor

@nat93 Everyone's work is important in this collaboration. Acting dismissive of others' work will not be tolerated here.

@nat93
Copy link
Contributor Author

nat93 commented Jun 30, 2025

@nat93 Everyone's work is important in this collaboration. Acting dismissive of others' work will not be tolerated here.

Hi @wdconinc , perhaps my message was not clear. The main idea is to optimize the procedure to avoid double work for everyone. I did not mean what you wrote. Opposite, I do respect other people work and it would be not efficient optimizing configuration that is already outdated, postponing the central detector protection and collider design - this is the message.

@nat93
Copy link
Contributor Author

nat93 commented Jul 4, 2025

Hi @ajentsch,
Thanks for catching the missing units — you are absolutely right that all dimensions should be explicitly defined with units. I have also added a few other missing units (including OMDs) and corrected some typos in the older scripts, as we previously discussed ('apperture' -> 'aperture').

@ajentsch
Copy link
Contributor

ajentsch commented Jul 4, 2025 via email

@johnlajoie
Copy link
Contributor

Hey @nat93 @simonge @ajentsch @wdconinc - I'm trying to follow along with this an other PR's to see where we are in converging on an updated geometry that can be used in a consistent fashion for the "events with background" that we need to get going on the preTDR studies. @simonge - you had a bulleted list of seven things you wanted to see before this PR was merged. Is this feasible in a reasonable amount of time, and how much time is that?

I realize a lot of work is being done by a small number of people so please don't take any of this as a criticism, just trying to to make sure I understand where we are.

@ajentsch
Copy link
Contributor

ajentsch commented Jul 7, 2025

EICrecon PR with updated OMD matrices is ready: eic/EICrecon#1957

@nat93
Copy link
Contributor Author

nat93 commented Jul 7, 2025

Dear @wdconinc, @simonge, and @johnlajoie,

Following the suggestions from Simon and Wouter, as well as our Zoom discussion, @ajentsch and I have refined the PR to meet the outlined criteria. - Many thanks to Alex for assistance.

The FWD beam pipe remains unchanged in this PR - there is no impact on the far-BWD detectors. On the FWD side, the beam pipe was adjusted to eliminate overlaps between the cryostat and the OMD chamber (Alex has updated the detector positions accordingly). The magnetic fields remain unchanged, and the detector acceptance is not reduced. In fact, on the FWD side, the acceptance may slightly increase due to the larger inner radius of the HSR cryostat compared to the beam stay-clear aperture.

This PR is a step forward, but additional improvements will follow: proper beam pipe definitions, a warm and split B2eR, and refinements to the magnetic fields and/or coil length (to be coordinated with the optics and magnet teams).

If everyone agrees, we propose to proceed with merging this PR.

@veprbl
Copy link
Member

veprbl commented Jul 8, 2025

Trying to figure out what are the remaining blockers.
@simonge You were planning to look at the impact of this latest version?
We also need to check material map before/after merging this.

@simonge
Copy link
Contributor

simonge commented Jul 8, 2025

From what I see there is no impact compared to the current version at the moment as expected.

@simonge
Copy link
Contributor

simonge commented Jul 8, 2025

We also need to check material map before/after merging this.
There shouldn't be any change to the material map as I understand because everything is outside of the central detector region and outside the beampipe in the FF/FB regions.

There might be some changes I suggest down the line but happy to approve now and get the OMD update in EICrecon merged too.

@nat93
Copy link
Contributor Author

nat93 commented Jul 9, 2025

Hi @simonge , should we expect or wait for any further checks? If not, can you please approve the PR so we can merge it? Thanks

Copy link
Contributor

@simonge simonge left a comment

Choose a reason for hiding this comment

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

Approved. I was hoping someone with more experience of the material map would confirm a new one wasn't needed.

@veprbl
Copy link
Member

veprbl commented Jul 9, 2025

I will take care of the map.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants