Skip to content

Conversation

@GuySten
Copy link
Contributor

@GuySten GuySten commented Oct 6, 2025

Description

Currently, when openmc load weight windows from an hdf5 file the meshes are not loaded.
This PR change this behavior so the meshes inside the weight windows hdf5 file are also loaded.

Fixes #3588

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format (version 15) 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)

@GuySten GuySten requested a review from pshriwise October 6, 2025 04:25
@GuySten GuySten marked this pull request as ready for review October 6, 2025 07:05
Copy link
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 for this, @GuySten. One comment on separating concerns. We'll also want to attempt some testing for this of course if you're up for taking a crack at that.

@GuySten
Copy link
Contributor Author

GuySten commented Oct 16, 2025

I've added tests for reading and writing mesh objects from HDF5.

@GuySten GuySten requested a review from pshriwise October 16, 2025 14:07
Copy link
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.

Relatively minor things to address here and then I think this is good! I refactored the object generation code because it was duplicated between the two read_meshes versions and if we ever update that I'd like it to be able to be changed in one place.

Thanks for adding to the C++ tests! I really like seeing that suite grow. I also added a test for a roundtrip of multiple meshes that I think should pass once one of the comments is addressed.

src/mesh.cpp Outdated
Comment on lines 1259 to 1260
fatal_error("Number of entries on <lower_left> must be the same "
"as the number of entries on <dimension>.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fatal_error("Number of entries on <lower_left> must be the same "
"as the number of entries on <dimension>.");
fatal_error("Number of entries on lower_left dataset must be the same "
"as the number of entries for the mesh dimension.");

Copy link
Contributor

Choose a reason for hiding this comment

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

There are other, similar error messages that imply the mesh is read from an XML file that we should probably update below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, we could factor out some of this logical checking into new methods for RegularMesh and RectilinearMesh like the CylindricalMesh::set_grid method does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I decided to refactor most of the logical checking.

@GuySten GuySten requested a review from pshriwise October 21, 2025 10:28
Copy link
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 for this added capability @GuySten! I'll merge once tests are green.

@yrrepy maybe try this out and let us know how it goes?

@pshriwise pshriwise enabled auto-merge (squash) October 21, 2025 16:38
@yrrepy
Copy link
Contributor

yrrepy commented Oct 21, 2025

Haven't given it a full shakedown, but it appears to do the task.

@pshriwise pshriwise merged commit c31032c into openmc-dev:develop Oct 21, 2025
24 of 26 checks passed
@GuySten GuySten deleted the meshes-from-hdf5 branch October 22, 2025 14:29
Grego01-biot pushed a commit to Grego01-biot/openmc that referenced this pull request Oct 27, 2025
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.

3 participants