Skip to content

Conversation

wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Jun 24, 2025

Briefly, what does this PR introduce?

This PR shrinks the world volume from 60 m x 60 m x 200 m to something that is just sufficient to enclose the relevant volumes. This way we won't be tracking slow neutrons on their way to death in air.

Needs:

What kind of change does this PR introduce?

  • Bug fix (issue: the world is too big)
  • 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: @ajentsch @simonge this makes the forward and backwards regions quite narrow, chosen based on what's currently in the geometry

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

No.

Does this PR change default behavior?

No.

@wdconinc
Copy link
Contributor Author

Hmm, "Unknown Shape TGeoCompositeShape" in convert-to-step...

@wdconinc
Copy link
Contributor Author

Removed volumes extruding out of mother volume by increasing some dimensions. Now looks like this in ZOY ortho view:
image

@github-actions github-actions bot added the topic: far-forward Deterctors for small-angle particles label Jun 24, 2025
@wdconinc
Copy link
Contributor Author

There was a hidden vacuum volume that is now also fixed, so finally overlaps succeed:
image

@wdconinc wdconinc requested review from ajentsch and simonge June 24, 2025 22:28
@wdconinc
Copy link
Contributor Author

For reference, this is the world before, which is clearly more than necessary (and it's all air, not vacuum):
image

@simonge
Copy link
Contributor

simonge commented Jun 25, 2025

Looks very useful, do you have any metrics which show the performance improvements?

It would be nice to be able to control the world volume more precisely for different configurations. Could the parameters be moved into a separate e.g. worlddefinition_default.xml file.

@wdconinc
Copy link
Contributor Author

Looks very useful, do you have any metrics which show the performance improvements?

It would be nice to be able to control the world volume more precisely for different configurations. Could the parameters be moved into a separate e.g. worlddefinition_default.xml file.

Not really benchmarked in a way that works with eic-shell. The way single-threaded Geant4 works, the only speed-up is what we gain from long-running tracks. There is an npsim plugin at eic/npsim#35 that prints the longest running tracks. Those go up to 100ms for neutrons that reach end of world, and there are typically a few of those per event (the other long duration tracks are optical photons bouncing in the DIRC).

The effect of the speed-up is potentially different for multi-threaded running. In that case the longest running track becomes a minimum duration for the event since it's the minimum unit of parallelism. So, while other short-running tracks can be parallelized, that doesn't work for the long-running tracks.

I don't think the speed-up is above the percent level, though, so I don't think there is too much benefit in tweaking this much. We could make it a compact file that is listed in the yml configs, but I would also want to be mindful of the potential for errors there when stuff extends outside the world.

@simonge
Copy link
Contributor

simonge commented Jun 25, 2025

I don't think the speed-up is above the percent level, though, so I don't think there is too much benefit in tweaking this much. We could make it a compact file that is listed in the yml configs, but I would also want to be mindful of the potential for errors there when stuff extends outside the world.

I would expected more significant gains in the tracking only geometries where there aren't many secondaries produced in the detector but lots might come from the air. When I was looking at this for the specific case of beamline tracking, killing the single electron track in the beampipe vacuum rather than allowing it to continue through 50m of air resulted in a 200x speed up. It won't be quite the same for this but might still be significant.

@wdconinc
Copy link
Contributor Author

Good point! Maybe sharpening up the world for those is useful then.

@veprbl veprbl reopened this Jun 25, 2025
@wdconinc wdconinc force-pushed the tighten-up-the-world branch from 35066f4 to cb82216 Compare July 16, 2025 15:01
@wdconinc wdconinc enabled auto-merge (squash) July 16, 2025 15:01
veprbl
veprbl previously approved these changes Jul 16, 2025
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.

Looks good to me.

@wdconinc wdconinc added this to the 25.08.0 milestone Jul 16, 2025
@wdconinc

This comment was marked as resolved.

@wdconinc wdconinc force-pushed the tighten-up-the-world branch from a337566 to 5f63390 Compare July 18, 2025 11:51
@wdconinc wdconinc requested a review from veprbl July 18, 2025 13:11
@github-actions github-actions bot added the topic: infrastructure Regarding build system, CI, CD label Jul 18, 2025
veprbl
veprbl previously approved these changes Jul 18, 2025
@veprbl veprbl changed the title feat: tighten up the world to a more minimal geometry feat: tighten up the world boundary, support acts 39 Jul 18, 2025
veprbl
veprbl previously approved these changes Jul 18, 2025
@github-actions github-actions bot added topic: barrel Mid-rapidity detectors topic: PID Particle identification labels Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: barrel Mid-rapidity detectors topic: far-forward Deterctors for small-angle particles topic: infrastructure Regarding build system, CI, CD topic: PID Particle identification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants