Add properties to settings w/ documentation, c++ loading of filename, and python round-trip test by gonuke · Pull Request #3808 · openmc-dev/openmc

gonuke · 2026-02-16T04:14:27Z

Description

This adds the path of the properties.h5 file as a new entry in the simulation settings. This file path can be used for loading properties from a file other than the default.

This will replace #3807 that has been withdrawn.

Fixes # (issue)

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)

openmc/settings.py

lewisgross1296

I like the settings approach, probably best to have it in one place, as opposed to every cell that has properties.

Might be a future issue/PR, but we should discuss how to handle if there are cells with instances in the properties file that are not distribcell in our current model. (e.g. programmatically added tallies from Cardinal)

with documentation and loading from c++ and python round trip test

gonuke · 2026-02-19T12:05:58Z

Hmmm... seems like my version of clang-format is too new and it is aligning comments differently that v15 (although the style guide does call for aligning comments)

lewisgross1296 · 2026-02-19T16:07:58Z

Recently, I've been installing clang-format-15 inside my docker image and formatting from the command line via clang-format-15 -i FILE

lewisgross1296 · 2026-02-19T20:02:01Z

Looked around / thinking about testing. It seems like this is the only OpenMC test that uses properties.h5.

Our tests could also take advantage of lib_init and export properties files with temperatures only/densities only/both. We could use the lattice_distribmat and lattice_distribrho tests as a base for the models that will have properties.

I can imagine the following tests being useful (and maybe more?)

import a property file that has both temperature/density and ask for both. Some kind of check that the cells have the temps we expect.
import a property file that only has temperature, ask for density, and test for the error
import a property file that has only density, ask for temperature, and test for the error
ask for temperature and density from a file that has neither, and test for both errors
ask for properties when no properties file exists

gonuke · 2026-02-20T01:02:02Z

Recently, I've been installing clang-format-15 inside my docker image and formatting from the command line via clang-format-15 -i FILE

That's what I get for trying to do this on my Mac :)

include/openmc/xml_interface.h

gonuke · 2026-02-22T19:48:01Z

It looks like I need to figure out how to place a dummy properties.h5 file in the tmpdir to test this - or back off ever further on testing this, noting that the existence of other files is not tested in unit tests

pshriwise · 2026-02-22T20:17:40Z

It looks like I need to figure out how to place a dummy properties.h5 file in the tmpdir to test this - or back off ever further on testing this, noting that the existence of other files is not tested in unit tests

This context managet may be of help

openmc/tests/__init__.py

Lines 8 to 25 in 83a30f6

    
           def cdtemp(files=None): 
        
               """Context manager to change to/return from a tmpdir. 
        
               Parameters 
        
               ---------- 
        
               files : Iterable of str or Path-like 
        
                   Set of files to copy into the temporary directory 
        
               """ 
        
               with tempfile.TemporaryDirectory() as tmpdir: 
        
                   cwd = os.getcwd() 
        
                   if files: 
        
                       for file in files: 
        
                           copy(file, tmpdir, follow_symlinks=True) 
        
                   try: 
        
                       os.chdir(tmpdir) 
        
                       yield 
        
                   finally: 
        
                       os.chdir(cwd)

gonuke · 2026-02-22T20:38:17Z

It looks like I need to figure out how to place a dummy properties.h5 file in the tmpdir to test this - or back off ever further on testing this, noting that the existence of other files is not tested in unit tests

This context manager may be of help

Perhaps, but it's already using a temp directory fixture. I could use both but not sure how they interact. I could also just use os to touch a file in the current temp dir.

The fixture is a little mysterious because it refers to a tmpdir but I can't find anywhere that tmpdir is defined??

pshriwise · 2026-02-22T22:07:58Z

Oh, I misunderstood. I thought you were trying to reference an already-present properties file. I don't quite understand why we need the file to exist though, it looks like that's only checked on the C++ side.

I think comparison of the str vs. Path types might be an issue. The value passed to the Settings.properties_file property is converted to a Path object in the call to input_path (if it isn't one already).

gonuke · 2026-02-22T22:27:25Z

I think comparison of the str vs. Path types might be an issue. The value passed to the Settings.properties_file property is converted to a Path object in the call to input_path (if it isn't one already).

Yeah - that's probably the problem. I was getting wrapped around on the type of the filename and not sure what will resolve to true.

pshriwise

Thanks @gonuke! I've included some minor comments here.

My comment on load order can be tackled in a separate PR.

include/openmc/capi.h

openmc/settings.py

src/material.cpp

pshriwise · 2026-02-23T18:27:51Z

src/summary.cpp

  // Read material properties
  for (const auto& mat : model::materials) {
-    mat->import_properties_hdf5(materials_group);
+    mat->import_properties_hdf5(


Commenting on what I think may be a pre-existing issue here and looking for guidance:

Above, we call Cell::set_density which populates the density multiplier vector based on the density of the material filling that cell. Here, we read the density of the material from the file, which may change the baseline density of the material from what it was at the time we called Cell::set_density and possibly invalidate the density multipliers of the cells? In a separate effort, should we perhaps change the order of these imports?

Looping @nuclearkevin in for a take as well.

A simple change to mitigate (but not entirely prevent) this is to have two read_density flags: read_cell_densities and read_material_densities, and document that one will override the other.

My preference would be to either read all domain densities or none and fix the ordering (again, in a separate PR if need be). My understanding is that some cells may have density multipliers and others may not, so the additional option could leave us in a confusing state where some densities are original and some from the properties file.

I misunderstood your early comment. I thought you were worried about one of them overwriting the other, but I see that the concern is more subtle.

openmc/settings.py

docs/source/io_formats/settings.rst

src/summary.cpp

lewisgross1296 · 2026-02-24T18:58:14Z

I think it's working!

 Importing properties from
 /home/lgross/cnerg/gcmr/multiphysics_depletion/BOL_mp_depl/180/properties.h5...
[openmc.deplete] t=0.0 s, dt=86400 s, source=3333333.3333333335

gonuke · 2026-02-24T21:44:16Z

I can't quite figure out why this test is failing? It seems like one of the python version tests was cancelled and that caused CI to be determined as failed. Can someone relaunch tests?

src/summary.cpp

include/openmc/capi.h

src/initialize.cpp

lewisgross1296 · 2026-03-05T17:15:11Z

openmc/settings.py

+    def _create_properties_file_element(self, root):
+        if self.properties_file is not None:
+            element = ET.Element("properties")
+            element.text = str(self.properties_file)


Something went wrong in the latest update @gonuke, I think it's here (i.e there's nothing called properties_file in the XML now.) I just re-tried and my settings looks like this

<settings> <properties>/home/lgross/cnerg/gcmr/multiphysics_depletion/BOL_mp_depl/180/properties.h5</properties> </settings>

which leads to this behavior

%%%%%%%%%%%%%%% %%%%%%%%%%%%%%%%%%%%%%%% %%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% %%%%%%%%%%%%%%%%%%%%%%%% %%%%%%%%%%%%%%%%%%%%%%%% ############### %%%%%%%%%%%%%%%%%%%%%%%% ################## %%%%%%%%%%%%%%%%%%%%%%% ################### %%%%%%%%%%%%%%%%%%%%%%% #################### %%%%%%%%%%%%%%%%%%%%%% ##################### %%%%%%%%%%%%%%%%%%%%% ###################### %%%%%%%%%%%%%%%%%%%% ####################### %%%%%%%%%%%%%%%%%% ####################### %%%%%%%%%%%%%%%%% ###################### %%%%%%%%%%%%%%%%% #################### %%%%%%%%%%%%%%%%% ################# %%%%%%%%%%%%%%%%% ############### %%%%%%%%%%%%%%%% ############ %%%%%%%%%%%%%%% ######## %%%%%%%%%%%%%% %%%%%%%%%%% | The OpenMC Monte Carlo Code Copyright | 2011-2025 MIT, UChicago Argonne LLC, and contributors License | https://docs.openmc.org/en/latest/license.html Version | 0.15.3-dev282 Commit Hash | 0e9eea39a8243b2223bb345d01fd091906a629f8 Date/Time | 2026-03-05 10:59:29 MPI Processes | 1 OpenMP Threads | 96 Reading model XML file 'model.xml' ... ERROR: Node "filepath" is not a member of the "properties" XML node

I think you either want to use element.setAttribute to set filepath or have the code just look for the properties element

I'm kind of confused why the test didn't fail though

Actually

// read properties from file if (check_for_node(root, "properties")) { properties_file = get_node_value(root, "properties"); if (!file_exists(properties_file)) { fatal_error(fmt::format("File '{}' does not exist.", properties_file)); } }

The test looks for a node called properties, which is why it works. Perhaps now we just call it properties instead of properties_file now that we only supply a file

Is your executable up to date?

nvm ~~I forgot to do make install after make and before installing the python api~~

EDIT: It's running now... I'll report back when it completes and hopefully it will be statistically equivalent to cardinal / OpenMC depletion. Also it printed

Importing properties from /home/lgross/cnerg/gcmr/multiphysics_depletion/BOL_mp_depl/180/properties.h5...

Any word here @lewisgross1296?

Still going :/ looks more promising so far. Probably will know by tomorrow

59/1 1.03796 1.04088 +/- 0.00175

lewisgross1296 reviewed Feb 16, 2026

View reviewed changes

openmc/settings.py Show resolved Hide resolved

lewisgross1296 suggested changes Feb 16, 2026

View reviewed changes

gonuke added 8 commits February 18, 2026 11:51

add properties file path to settings

8b1b8dc

with documentation and loading from c++ and python round trip test

add flags to indicate which properties to read from properties file

8713fe9

Add new XML reading method to check existence of file path

76c258e

add flags for which properties to read from file

0a7636b

read properties file after other initialization

9942d78

propagate boolean for property reads into C API and related python calls

886c641

propagate read properties flags to cells

fd1594a

propagate read properties flags to material

5852ec0

gonuke force-pushed the properties_in_settings branch from 5aeb316 to 5852ec0 Compare February 19, 2026 01:29

gonuke mentioned this pull request Feb 19, 2026

Load cell temperatures from HDF5 file indicated by new XML element #3807

Closed

5 tasks

gonuke added 2 commits February 19, 2026 06:28

add new booleans to header

a9b3f28

apply clang formatting

1b78b84

gonuke added 8 commits February 19, 2026 20:19

fix formatting

cc10db6

manually fix clang-format inconsistency

cf9a27e

fix string to char* conversion

4f796fb

declare new XML reader

6a862b6

add missing semi-colon and guess at formatting

cfcbfc2

add default value to signature

fb22bba

clang-format-15

9a20d6e

add file_utils to xml_interface

015df87

gonuke commented Feb 20, 2026

View reviewed changes

include/openmc/xml_interface.h Outdated Show resolved Hide resolved

gonuke added 3 commits February 21, 2026 10:31

back away from new XML reader for valid file paths

d8c92fd

Improve documentation

99ca76f

switch to C++ error handling

a86f00e

add temporary file for testing

902e749

gonuke added 2 commits February 22, 2026 16:21

don't convert back to string in test

a62b545

Don't generate file, but do test against Path

c7a8090

gonuke marked this pull request as ready for review February 23, 2026 00:49

gonuke requested a review from paulromano as a code owner February 23, 2026 00:49

gonuke requested a review from pshriwise February 23, 2026 00:49

pshriwise requested changes Feb 23, 2026

View reviewed changes

lewisgross1296 reviewed Feb 23, 2026

View reviewed changes

src/summary.cpp Outdated Show resolved Hide resolved

gonuke added 5 commits February 23, 2026 13:21

abbreviate names

ebbdc1a

update documentation

6355305

cleanup comments and error messages

19abae7

simplify early exit logic

1726602

style update

0e9eea3

GuySten reviewed Mar 2, 2026

View reviewed changes

src/summary.cpp Outdated Show resolved Hide resolved

pshriwise reviewed Mar 3, 2026

View reviewed changes

include/openmc/capi.h Outdated Show resolved Hide resolved

lewisgross1296 reviewed Mar 4, 2026

View reviewed changes

src/initialize.cpp Outdated Show resolved Hide resolved

read the properties in the right place

41317cf

gonuke force-pushed the properties_in_settings branch 2 times, most recently from aaee4fc to a0ace4e Compare March 5, 2026 12:06

unwind fine-grained import booleans

cde42e1

gonuke force-pushed the properties_in_settings branch from a0ace4e to cde42e1 Compare March 5, 2026 12:07

gonuke requested a review from pshriwise March 5, 2026 12:08

lewisgross1296 reviewed Mar 5, 2026

View reviewed changes

Conversation

gonuke commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Uh oh!

lewisgross1296 left a comment

Choose a reason for hiding this comment

Uh oh!

gonuke commented Feb 19, 2026

Uh oh!

lewisgross1296 commented Feb 19, 2026

Uh oh!

lewisgross1296 commented Feb 19, 2026

Uh oh!

gonuke commented Feb 20, 2026

Uh oh!

Uh oh!

gonuke commented Feb 22, 2026

Uh oh!

pshriwise commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gonuke commented Feb 22, 2026

Uh oh!

pshriwise commented Feb 22, 2026

Uh oh!

gonuke commented Feb 22, 2026

Uh oh!

pshriwise left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lewisgross1296 commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gonuke commented Feb 24, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lewisgross1296 Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lewisgross1296 Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

gonuke commented Feb 16, 2026 •

edited

Loading

pshriwise commented Feb 22, 2026 •

edited

Loading

lewisgross1296 commented Feb 24, 2026 •

edited

Loading

lewisgross1296 Mar 5, 2026 •

edited

Loading

lewisgross1296 Mar 5, 2026 •

edited

Loading