Skip to content

Limit DAGMC ray fire distance to collision point for BVH pruning#3961

Open
eckertben wants to merge 2 commits into
openmc-dev:developfrom
eckertben:bvh-ray-dist-cap
Open

Limit DAGMC ray fire distance to collision point for BVH pruning#3961
eckertben wants to merge 2 commits into
openmc-dev:developfrom
eckertben:bvh-ray-dist-cap

Conversation

@eckertben
Copy link
Copy Markdown

Description

Please include a summary of the change and which issue is fixed if applicable. Please also include relevant motivation and context.

Limit the ray fire distance in DAGMC geometry to the sampled collision distance. When a collision distance is sampled before boundary finding, there is no need to search for boundaries beyond that distance since the collision event will be selected regardless. Passing this distance as a cap to the DAGMC ray fire allows the BVH traversal to prune nodes beyond the collision point, resulting in more efficient particle tracking.

Local Tests:
-All pass

Tests on ATR benchmark model provided by Dr. Shriwise (100 batches, 10k particles per batch):

  • Baseline active tracking rate: 656.7 particles/second
  • Modified active tracking rate: 661.9 particles/second
  • k-effective unchanged

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 18) on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@eckertben eckertben requested a review from pshriwise as a code owner June 5, 2026 18:47
Copy link
Copy Markdown
Contributor

@pshriwise pshriwise left a comment

Choose a reason for hiding this comment

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

Thanks @eckertben! With some more testing this may unlock some additional performance boosts for well-formed DAGMC models.

Would you mind exposing this as an optional behavior that's marked on the DAGMCUniverse object itself? We'll want to maintain the option for users to run with the current behavior in the case they don't have confidence that the DAGMC surface mesh is well-formed (i.e. successfully imprinted & merged and watertight)

@gonuke
Copy link
Copy Markdown
Contributor

gonuke commented Jun 5, 2026

FWIW, I think the early MCNP implementation of DAGMC did just this. There are problems, however, as @pshriwise notes when there are imperfections in the DAGMC model.

@pshriwise
Copy link
Copy Markdown
Contributor

FWIW, I think the early MCNP implementation of DAGMC did just this. There are problems, however, as @pshriwise notes when there are imperfections in the DAGMC model.

Indeed! I thought we'd try it out here as well and see what it gets us.

@pshriwise
Copy link
Copy Markdown
Contributor

I tried this out tonight and I think we're missing the final piece where we pass the distance limit into the DAGMC ray_fire call within DAGCell:distance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants