Support for a single `model.xml` file by pshriwise · Pull Request #2291 · openmc-dev/openmc

pshriwise · 2022-11-04T04:57:29Z

This PR adds the ability to read a single input XML, model.xml as input.

If this file is present, OpenMC will read the settings, materials, geometry, tallies, and plots from this file and ignore any other XML files present in the directory -- a warning will be displayed if those additional XMLs are present. If a model.xml file is absent, OpenMC will fall back to using the current set of separate XML files we rely on for input.

In the Python API, a separate_xmls keyword argument has been added to the openmc.Model export_to_xml and from_xml methods to support both reading and writing a single XML file or separate XML files. This should make it easy to either stick with the current workflow for input generation with multiple XML files or convert old inputs into a single model.xml file.

This PR is still a little rough (needs a fresh look in the morning and several tests), so I'm marking it as a draft for now.

shimwell · 2022-11-17T05:03:16Z

This looks like the way to go, nice work Patrick. Perhaps this closes #1906?

paulromano

Thanks for putting this together @pshriwise!

include/openmc/initialize.h

openmc/geometry.py

openmc/model/model.py

tests/regression_tests/model_xml/test.py

tests/unit_tests/test_model.py

openmc/model/model.py

pshriwise · 2022-11-28T16:55:43Z

With the additional changes to exec.py to support custom file names, I’d like to add a couple more tests to ensure that works correctly.

paulromano

Thanks for the updates @pshriwise! Another round of comments:

openmc/model/model.py

paulromano · 2022-12-06T13:26:21Z

openmc/model/model.py

+        directory : str
+            Directory to write XML files to. If it doesn't exist already, it
+            will be created.


Rather than having separate directory and filename arguments, perhaps we should have a single path argument. When exporting separate XMLs, it would expect a directory. When exporting a single model.xml, the path could either be a directory (in which case 'model.xml' will be appended) or an explicit filename.

paulromano · 2022-12-06T13:28:58Z

src/initialize.cpp

+  auto other_inputs = {"materials.xml", "geometry.xml", "settings.xml", "tallies.xml", "plots.xml"};
+  for (const auto& input : other_inputs) {
+    if (file_exists(settings::path_input + input)) {
+      warning(("Other XML file input(s) are present. These file will be ignored in favor of the model.xml file."));


Comment remains about need for clang-formatting

src/initialize.cpp

tests/regression_tests/model_xml/test.py

… to make sure I don't break stuff.

…ssary.

@paulromano

Incorporating suggestions from @paulromano Co-authored-by: Paul Romano <paul.k.romano@gmail.com>

…he python exec

…path_input

Co-authored-by: Paul Romano <paul.k.romano@gmail.com>

pshriwise · 2022-12-07T14:11:08Z

openmc/model/model.py


-    def export_to_xml(self, directory='.', remove_surfs=False):
-        """Export model to XML files.
+    def export_to_xml(self, directory='.', remove_surfs=False, separate_xmls=True, path='model.xml'):


@paulromano I'm wondering if we should take the same approach here as we did for from_xml (i.e. maintain the current export_to_xml method and add an export_to_model_xml method alongside it for now). Thoughts?

Yeah, in light of the changes for from_xml, I think it would make sense to keep this as two separate methods as well. Thanks!

pshriwise · 2022-12-07T16:35:08Z

A couple of changes related to filesystem functions are worth noting in the latest set of updates:

A dir_exists function was added to file_utils.h. It uses the C lib system calls and flags to check whether or not a path is a directory. Once we switch to C++17, we can use the std::filesystem component there instead.
Our file_exists method in file_utils.h will return true for existing directories as well as files. A dir_exists call for the input filepath has been added to this method so it returns false for directories.

openmc/executor.py

openmc/model/model.py

openmc/_xml.py

openmc/executor.py

openmc/material.py

paulromano · 2022-12-12T22:05:21Z

openmc/model/model.py

+        if model.settings.entropy_mesh is not None:
+            meshes[model.settings.entropy_mesh.id] = model.settings.entropy_mesh


There's also the UFS mesh (this feels a little brittle -- wondering if there's a better way we can capture all the meshes from settings.xml)

Suggested change

if model.settings.entropy_mesh is not None:

meshes[model.settings.entropy_mesh.id] = model.settings.entropy_mesh

if model.settings.entropy_mesh is not None:

meshes[model.settings.entropy_mesh.id] = model.settings.entropy_mesh

if model.settings.ufs_mesh is not None:

meshes[model.settings.ufs_mesh.id] = model.settings.ufs_mesh

~~Yeah maybe we should consolidate all of the mesh writing into a single method?~~

☝🏻 misunderstanding on my part

How about an internal attribute on the class that tracks meshes used (updated inside the property setters). Then this would become:

meshes = {mesh.id: for mesh in model.settings._meshes}

paulromano · 2022-12-12T22:07:21Z

openmc/settings.py

                subelement.text = str(value)

-    def _create_entropy_mesh_subelement(self, root):
+    def _create_entropy_mesh_subelement(self, root, mesh_memo=None):


Need similar changes for _create_ufs_mesh_subelement

src/initialize.cpp

tests/regression_tests/model_xml/test.py

Co-authored-by: Paul Romano <paul.k.romano@gmail.com>

Several small fixes

pshriwise marked this pull request as ready for review November 10, 2022 16:32

pshriwise requested a review from amandalund as a code owner November 10, 2022 16:32

pshriwise force-pushed the model-xml branch from 1a6186d to d0a799c Compare November 11, 2022 04:44

paulromano requested changes Nov 22, 2022

View reviewed changes

paulromano requested changes Dec 6, 2022

View reviewed changes

pshriwise and others added 23 commits December 6, 2022 13:03

Factor out to_xml_element methods for XML classes

2555b95

Writing all main nodes to a single XML file

ad746cb

Writing all output under a single root node

a09a61e

Keeping old option to write separate XMLs so I can still use to tests…

7a9d8c9

… to make sure I don't break stuff.

Adding signatures for reading information from an XML node where nece…

0409652

…ssary.

Correcting doc scope and some reads

cdd2a6b

Refactoring model read into it's own function.

bec9bf6

Factoring out some common lines between reader functions

81aa653

Adding import for single or multiple XMLs

9391f17

Fix loop variable overwrite

af6b0af

Properly call read_plots_xml

5ac6e87

Other bug fixes

2e71658

Removing walrus operator

b4dcc30

Restructureing input function calls a little

611475a

Adding initial test for exporting model.xmls. Correcting material maps

021bb28

Correcting doc strings for material mapping sent to

abd3d51

Running some existing regression tests using single XML file

662d83a

Making sure meshes are only written once to a model.xml file

1e7dbe9

Apply suggestions from code review

d034e1d

Incorporating suggestions from @paulromano Co-authored-by: Paul Romano <paul.k.romano@gmail.com>

Addressing some comments from PR

b76825f

Adding ability to specify an input filename to the executable or to t…

d984716

…he python exec

Test comment and updated error messages

366f9e9

Addressing a few more comments in C++ code

1e6cb68

pshriwise added 8 commits December 6, 2022 13:03

Updates to import/export methods suggested by @paulromano

cd7c3bf

Updating handling of single XML input to leverage existing settings::…

ef2f690

…path_input

Moving Model.from_separate_xmls back to Model.from_xml.

574845b

Adding function to check if a path is a directory

270331a

Updates to simplify model filename checking

bc1791d

Changing name of input argument for Python executor

5634d83

Docstring correction

d639d1d

Updating tests to reflect changes to API

939adbe

pshriwise force-pushed the model-xml branch from c8b1ad4 to 939adbe Compare December 6, 2022 20:41

pshriwise and others added 3 commits December 6, 2022 21:49

Some cleanup of file utils

7fdfed2

Small improvements to read_model_xml

55d77cf

Update tests/regression_tests/model_xml/test.py

7873b4d

Co-authored-by: Paul Romano <paul.k.romano@gmail.com>

pshriwise commented Dec 7, 2022

View reviewed changes

pshriwise added 2 commits December 7, 2022 08:28

Some housekeeping in the Python classes

ef13643

Correct use of mesh memo in single XML export

9c942db

pshriwise commented Dec 7, 2022

View reviewed changes

openmc/executor.py Show resolved Hide resolved

Renaming and docstring/comment updates

916863f

paulromano requested changes Dec 12, 2022

View reviewed changes

paulromano mentioned this pull request Dec 12, 2022

allows Geometry.from_xml() to accept path for materials file #2326

Closed

pshriwise and others added 6 commits December 12, 2022 20:23

Apply @paulromano suggestions from code review

145594e

Co-authored-by: Paul Romano <paul.k.romano@gmail.com>

Addressing comments from @paulromano

f3bcad3

Improve comment on recursion arguments

43687e5

Adding mesh memo to Settings.from_xml_element

2eca57a

Several small fixes

25f2bd5

Merge pull request #16 from paulromano/model-xml-pr

64aee3b

Several small fixes

paulromano approved these changes Dec 24, 2022

View reviewed changes

paulromano merged commit 3f8f8f6 into openmc-dev:develop Dec 24, 2022

shimwell mentioned this pull request Dec 24, 2022

allowing xml file for materials #2333

Merged

paulromano mentioned this pull request Jun 21, 2024

Duplicate mesh writing in XML files. #1906

Closed

		if model.settings.entropy_mesh is not None:
		meshes[model.settings.entropy_mesh.id] = model.settings.entropy_mesh

Conversation

pshriwise commented Nov 4, 2022

Uh oh!

shimwell commented Nov 17, 2022

Uh oh!

paulromano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pshriwise commented Nov 28, 2022

Uh oh!

paulromano left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulromano Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

paulromano Dec 6, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pshriwise Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

paulromano Dec 12, 2022

Choose a reason for hiding this comment

Uh oh!

pshriwise Dec 13, 2022

Choose a reason for hiding this comment

Uh oh!

pshriwise commented Dec 7, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paulromano Dec 12, 2022

Choose a reason for hiding this comment

Uh oh!

pshriwise Dec 13, 2022 • 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!

paulromano Dec 12, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pshriwise Dec 13, 2022 •

edited

Loading