Skip to content

Allow mesh material volume calculations outside model geometry - #4028

Open
paulromano wants to merge 8 commits into
openmc-dev:developfrom
paulromano:mmv-outside-geometry
Open

Allow mesh material volume calculations outside model geometry#4028
paulromano wants to merge 8 commits into
openmc-dev:developfrom
paulromano:mmv-outside-geometry

Conversation

@paulromano

Copy link
Copy Markdown
Contributor

Description

Updates Mesh.material_volumes to support meshes that extend beyond the model geometry. During ray tracing, regions outside the geometry are simply treated as void material. This enables a user to, for example, use Cartesian meshes to cover geometries with a curved outside boundary (sphere, cylinder). The implementation follows the ray-traced plotting approach for locating model boundaries from undefined space (utilizing advance_to_boundary_from_void). Tests have been updated, including a comparison against an equivalent geometry with an explicit enclosing void cell.

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)

@paulromano
paulromano requested a review from pshriwise as a code owner August 29, 2026 05:10

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for supporting this additional scenario @paulromano! The refactor of the algorithm is quite nice too. A few questions here but no major concerns from me.

Comment thread include/openmc/mesh.h

//! Determine volume of materials within each mesh element
//!
//! Portions of mesh elements outside the model geometry are treated as void.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a hypothetical, but if one wanted a "background" material homogenized into mesh elements exterior to the model could one add an exterior cell outside the existing model with such a material and expect that it is homogenized in such elements?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, I don't see any reason that wouldn't work. Only regions that are outside all root universe cells are assigned void.

Comment thread src/dagmc.cpp
// assigned. In both cases, return an infinite distance and invalid surface
// index rather than marking a particle as lost.
if (settings::run_mode == RunMode::PLOTTING ||
p->lowest_coord().cell() == C_NONE)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks reasonable, but our raytrace plots handle a similar scenario without this modification. I wonder why that is. I'm guessing an extra find_cell may be performed somewhere?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking at the current algorithm for computing mesh material volumes, I'm a little confused. If the particle is located right after being source from site in a model with a DAGMC universe as its root universe, then it should wind up in the implicit complement right? Probably missing something here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is essentially correct. Ray::trace has an initial phase that calls exhaustive_find_cell, repeatedly calls advance_to_boundary_from_void, and then calls exhaustive_find_cell again after each candidate model boundary. The plotting path treats a no-hit result while probing root cells as an infinite distance rather than as a lost particle, and stops when there is no further model surface. The MMV calculation now follows the same outside-to-boundary search pattern but it additionally scores each interval outside the model as void.

Regarding the second comment, the implicit complement fallback looks like it only applies when a DAGMC universe is within another one (see DAGUniverse::find_cell).

Comment thread src/geometry.cpp
bool neighbor_list_find_cell(GeometryState& p, bool verbose)
{

#ifdef OPENMC_DAGMC_ENABLED

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems perfectly valid to place this check here too, but I'm curious as to why this needed to be moved out of the Particle::cross_surface method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that's a good question and I probably should have preemptively explained this. The old location only covered the actual transport code path. Both Ray::trace and Mesh::material_volumes perform their own surface crossing and call neighbor_list_find_cell directly, so they bypassed Particle::cross_surface. That allowed stale DAGMC facet history to survive when a ray crossed a CSG surface into another instance of the same DAGMC universe (@jtramm surfaced this in a review with Claude Code). The reset is now centralized in neighbor_list_find_cell, which is the common point reached after these CSG crossings. This preserves the transport behavior while also covering raytrace plots and MMV.

Comment thread src/mesh.cpp
// Add the contribution from a ray segment. The positions used here are
// kept separate from the particle position because the latter is moved a
// tiny distance across each surface for robust geometry searches.
auto add_segment = [&](const Position& r0, const Position& r1,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This method makes me wonder how close we are to merging this capability with the raytrace plots algorithm.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I would definitely like to go in this direction. I think we're getting pretty close!

"""Test a mesh extending outside a root DAGMC universe."""
openmc.reset_auto_ids()

dagmc_path = (Path(__file__).parents[1] /

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we need some suite-wide fixtures for the DAGMC models. They're used in many places now. Not asking this of you here, but if you think it would be useful maybe make an issue and I can tackle that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed! Several tests now construct models from the same DAGMC fixtures, so a suite-wide fixture/helper would be wise. Since you're volunteering, I'll let you handle it as a follow up 😄

@paulromano

Copy link
Copy Markdown
Contributor Author

@pshriwise I've responded to all your comments. Let me know if you think anything needs updating. @jtramm I've incorporated fixes for the issues that we discussed off line. Thanks to both of you for your time reviewing this!

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.

2 participants