Python source class refactor #2524

pshriwise · 2023-05-16T03:34:15Z

The Source class currently holds attributes that allow it to specify sources that contain independent distributions to be sampled, file sources, and custom/compiled source libraries to be called to generate particles.
On the C++ side, we have three classes that inherit from a common Source class: IndependentSource, FileSource, and CustomSource.

These changes refactor the Python API's source classes so that there are corresponding classes to those in the C++ interface. Source has been renamed to IndependentSource and a new base class, Source has been added to the module. Three classes inherit from Source : IndpendentSource, FileSource, and CompiledSource. (I renamed CustomSource to CompiledSource in both the C++ and Python code since I think that name is more clear).

This name change from Source to IndependentSource is going to cause a lot of updates to user scripts/code. With that being, such a hassle I'd understand if we don't decide to keep that change just for the sake of parity between the C++ and Python classes. cue revival of a discussion about PyBind/automated wrapping here

In terms of file specs, sources can now be identified by a type attribute on the XML element, but interestingly the logic in C++ for how these sources are created still holds, so old settings.xml files should still work with the OpenMC executable if we don't change that code. This is not true for XML files, however. Older XML files can no longer be read into Python objects unless we use similar logic to that of the C++ code, which would make the type attribute just a placeholder for information. I'd be interested in others' thoughts here.

…updated too

gonuke

Thanks for this clear improvement @pshriwise. A few comments on readability and the test for compiled sources.

tests/regression_tests/adj_cell_rotation/inputs_true.dat

gonuke · 2023-05-16T12:51:11Z

openmc/source.py

+    Attributes
+    ----------
+    type : str
+        Indicator of source type. One of ('source', 'mesh', 'file', 'compiled')


It seems redundant and potentially confusion to declare a Source as type 'source'. Perhaps choose something other than 'source' for the default type. Maybe 'analytic', 'computed', ???

Fair point. I think matching the class name here makes sense ("independent"). If we feel that this name isn't sufficiently descriptive then perhaps we should change both.

I also need to remove "mesh" from the list here. These changes were initially added on top of my branch adding a mesh source class..

openmc/source.py

tests/regression_tests/source_dlopen/inputs_true.dat

gonuke · 2023-05-16T13:03:58Z

tests/regression_tests/source_parameterized_dlopen/inputs_true.dat

@@ -18,6 +18,6 @@
    <particles>1000</particles>
    <batches>10</batches>
    <inactive>0</inactive>
-    <source library="build/libsource.so" parameters="1e3" strength="1.0"/>
+    <source strength="1.0" type="source"/>


This source should still have a 'library' member, if I understand the changes to source.py correctly.

…ted results are not null.

…ing error is raised when attrs are set on existing instance

gonuke

There's a lingering inconsistency in the string to be used for the type of the independent source. Wonder if that should be declared once at a high scope and reused?

gonuke · 2023-05-18T01:44:49Z

docs/source/io_formats/settings.rst

@@ -449,24 +449,29 @@ attributes/sub-elements:

    *Default*: 1.0

+  :type:
+    Indicator of source type. One of ``independent_source``, ``file``, or ``compiled``.


The files below only have type='independent' - either this documentation or those files should change...

The other classes use the unique component of the class name as their type, so we should do the same here. Updating that docstring now.

openmc/source.py

docs/source/io_formats/settings.rst

paulromano

Thanks for this PR @pshriwise! In addition to the line comments, please search through the documentation (*.rst) for the needed name changes (openmc.Source as well as anything related to custom sources).

docs/source/io_formats/settings.rst

openmc/source.py

tests/regression_tests/source_dlopen/test.py

tests/regression_tests/unstructured_mesh/test.py

tests/unit_tests/test_source.py

Co-authored-by: Paul Romano <[email protected]>

…id source type when reading from XML

… write particle type to XML.

openmc/source.py

tests/regression_tests/source_dlopen/test.py

openmc/source.py

Co-authored-by: Paul Romano <[email protected]>

…flect this.

…write

paulromano

Two more small changes and then this is good to go!

openmc/source.py

paulromano · 2023-06-14T11:39:04Z

openmc/source.py

+    @property
+    def file(self):
+        return self._file
+
+    @file.setter
+    def file(self, filename: str):
+        cv.check_type('source file', filename, str)
+        self._file = filename


The argument in the constructor is filename but the property is file. Since this is a new class, let's pick one and be consistent. In fact, it might be better to call it path and make sure it accepts pathlib.Path objects.

Co-authored-by: Paul Romano <[email protected]>

paulromano

A few more little fixes needed:

openmc/settings.py

openmc/source.py

Co-authored-by: Paul Romano <[email protected]>

paulromano · 2023-06-21T02:28:03Z

Thanks again @pshriwise!

pshriwise added 8 commits May 12, 2023 23:43

Refactor of Python source classes and rename one class. Test outputs …

8615975

…updated too

Updating track output results

393fa1b

Updating ncrystal inputs

22a9727

Renaming SourceBase --> Source and Source --> IndependentSource

7d7942b

Adding tests for deprecated parameters.

f6a5626

Removing MeshSource class and other small updates.

60ef091

Fixing indentation from name Source --> IndependentSource name change

1fcf9d9

updating source type

fc18fbf

pshriwise requested review from amandalund and drewejohnson as code owners May 16, 2023 03:34

gonuke suggested changes May 16, 2023

View reviewed changes

pshriwise added 7 commits May 16, 2023 11:49

Update type value to 'independent' for IndependentSource class

6e2a1d9

Correcting class type in dlopen tests and adding a tally so the expec…

460a412

…ted results are not null.

Updating error type for file, library, parameter attributes and ensur…

5f5c8e8

…ing error is raised when attrs are set on existing instance

Resetting test binaries

5fecafa

Updating libmesh regression test inputs

4ff3c1b

Filling out docstrings

1b7d68d

Updating the IO spec

d207cdf

gonuke suggested changes May 18, 2023

View reviewed changes

Correcting docs for IndependentSourse type attr value.

70ee07f

cfichtlscherer reviewed May 19, 2023

View reviewed changes

docs/source/io_formats/settings.rst Outdated Show resolved Hide resolved

fix typo

e363c73

paulromano requested changes Jun 9, 2023

View reviewed changes

pshriwise and others added 6 commits June 9, 2023 15:29

Apply suggestions from @paulromano

0e46280

Co-authored-by: Paul Romano <[email protected]>

Renaming Source to SourceBase

c11abe1

Make populate_xml_element an abstractmethod. Add ValueError for inval…

3027d8b

…id source type when reading from XML

Alternative method for protecting misuse of IndependentSource. Always…

979e803

… write particle type to XML.

Removing old setters. Passing parameters

56dbe88

Adding deprecated Source class for bkw compatibility.

2516bec

paulromano requested changes Jun 11, 2023

View reviewed changes

pshriwise and others added 8 commits June 12, 2023 00:07

Apply suggestions from @paulromano

798f351

Co-authored-by: Paul Romano <[email protected]>

Updating source energy and test results

265b34d

Correcting default getattr behavior and adding a test check

e40da02

Removing additional IndependentSource parameter. Updating tests to re…

2b2e576

…flect this.

Updating regression tests expected inputs for explicit particle type …

9d2ba25

…write

Updating unstructured mesh test inputs

7b30736

Merge remote-tracking branch 'upstream/develop' into src-refactor

b290125

Updating ncrystal test inputs

0734546

paulromano requested changes Jun 14, 2023

View reviewed changes

pshriwise and others added 9 commits June 15, 2023 23:18

CI trigger

fa01b54

Formatting dlopen source test code

c1d4e91

Merge remote-tracking branch 'upstream/develop' into src-refactor

f09505e

Putting setters back in their places

19fd7a7

Reordering source properties and removing duplicates

67d9f0d

FileSource.file to FileSource.path

1f50bbf

Apply suggestion from @paulromano

9e24f7a

Co-authored-by: Paul Romano <[email protected]>

Updating docstring and some more tests.

8648f3c

Changing test attribute back

f5ebea2

paulromano requested changes Jun 20, 2023

View reviewed changes

pshriwise and others added 2 commits June 20, 2023 07:22

Apply suggestions from @paulromano

a2e09c6

Co-authored-by: Paul Romano <[email protected]>

Removing duplicate code block and moving setters

fe23e7a

paulromano approved these changes Jun 21, 2023

View reviewed changes

paulromano merged commit eda39ad into openmc-dev:develop Jun 21, 2023

shimwell mentioned this pull request Jun 21, 2023

openmc.Source now renamed to openmc.IndependentSource fusion-energy/openmc-plasma-source#80

Closed

pshriwise deleted the src-refactor branch June 21, 2023 19:11

pshriwise mentioned this pull request Jun 21, 2023

Updating source definitions in examples #2571

Merged

3 tasks

shimwell mentioned this pull request Jun 22, 2023

version 0.13.4 of openmc API changes Source() CompiledSource() open-radiation-sources/parametric-plasma-source#34

Open

paulromano mentioned this pull request Sep 1, 2023

Update documentation around use of source classes #2679

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python source class refactor #2524

Python source class refactor #2524

pshriwise commented May 16, 2023

gonuke left a comment

gonuke May 16, 2023

pshriwise May 16, 2023 •

edited

Loading

gonuke May 16, 2023

gonuke left a comment

gonuke May 18, 2023

pshriwise May 18, 2023

paulromano left a comment

paulromano left a comment

paulromano Jun 14, 2023

paulromano left a comment

paulromano commented Jun 21, 2023

Python source class refactor #2524

Python source class refactor #2524

Conversation

pshriwise commented May 16, 2023

gonuke left a comment

Choose a reason for hiding this comment

gonuke May 16, 2023

Choose a reason for hiding this comment

pshriwise May 16, 2023 • edited Loading

Choose a reason for hiding this comment

gonuke May 16, 2023

Choose a reason for hiding this comment

gonuke left a comment

Choose a reason for hiding this comment

gonuke May 18, 2023

Choose a reason for hiding this comment

pshriwise May 18, 2023

Choose a reason for hiding this comment

paulromano left a comment

Choose a reason for hiding this comment

paulromano left a comment

Choose a reason for hiding this comment

paulromano Jun 14, 2023

Choose a reason for hiding this comment

paulromano left a comment

Choose a reason for hiding this comment

paulromano commented Jun 21, 2023

pshriwise May 16, 2023 •

edited

Loading