Allowing multiple cells in surface source banking

[MohamedElkamash]

Description

Currently, the surf_source_write setting allows the use of the cell, cellfrom, or cellto parameters to filter particles banked in the surface source. Recently, there has been interest in extending this behavior to allow filtering by multiple cells instead of a single cell. The main objective of this development is to enable filtering by multiple cells, where each cell can have an associated direction: "to", "from", or "both".

Motivation

In the simple model shown below, we are interested in banking particles that cross surfaces 2, 4, 9, or 10 and enter either cell 6 or cell 7. This behavior cannot be achieved with the current design because the cellto parameter accepts only a single cell ID.

New feature

A new cells argument has been added to the surf_source_write setting to allow specifying multiple cells for particle banking. An optional directions argument can be provided to define the corresponding direction for each cell listed in cells. If directions is not specified, the default behavior is "both" for every cell in cells. For example, the case above can now be implemented as:

settings.surf_source_write = {
	'max_particles': 5000,
	'surface_ids': [2, 4, 9, 10],
	'cells': [6, 7],
	'directions': ["to", "to"]
}

Handling duplicate cells

Although duplicate entries in cells are discouraged (since equivalent behavior can typically be expressed using "both"), they are handled safely. If duplicate cell entries are provided with different directions, their directions are combined using a union rule. For example:

settings.surf_source_write = {
	'max_particles': 5000,
	'surface_ids': [1, 2],
	'cells': [6, 7, 7],
	'directions': ["to", "to", "from"]
}

is equivalent to:

settings.surf_source_write = {
	'max_particles': 5000,
	'surface_ids': [1, 2],
	'cells': [6, 7],
	'directions': ["to", "both"]
}

Tests

Two regression tests were added based on the model described above:

1. Sum test

Verifies that filtering by multiple cells produces the same result as summing the results of individual simulations. Specifically, the number of particles entering cell 6 in one simulation plus the number entering cell 7 in a second simulation is equal to the number of particles entering cells 6 and 7 in a single simulation.

2. Duplicate equivalence test

Verifies that different duplicate combinations in cells produce identical surface source outputs when their effective directions are equivalent.

Backward compatibility

The original syntax (cell, cellto, cellfrom) remains fully supported. However, it cannot be used simultaneously with the new syntax (cells and directions). If both syntaxes are provided, an error is raised.

Additional changes

Previously, the logic for rejecting surfaces from being added to the surface source bank was partially implemented in the Surface constructor and partially during the simulation, depending on whether surface_ids was specified. This logic has now been fully moved to the Surface constructor.

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)

[JoffreyDorville]

Thanks for the PR @MohamedElkamash, this is a really nice addition to the surface source write feature!

The logic behind surf_source_ is way more readable now and the way duplicates are managed in the user inputs is an elegant solution. Also, this new feature being compatible with the previous syntax is a big plus I think. It will let us time to warn users that we will adopt this new syntax in the future.

I made some modifications directly as they are not changing the logic of your implementation. Can you take a look at the new commits and tell me what you think?

Here is a summary of the main things I modified:

• updated error messages + docs,
• added unit tests,
• removed unused settings,
• restored the check that tests if the bank is full at the beginning of add_surf_source_to_bank() to avoid going through the logic if the bank is already full.

Regarding the regression tests, test_surface_source_multiple_cells() might be redundant with test_surface_source_cell_history_based() in the sense that you can easily confirm at the creation of the new cases (22, 23, 24) that the number of particles stored is consistent. If something were to break this behavior, we would see a change with test_surface_source_cell_history_based(). I would suggest to remove test_surface_source_multiple_cells().

For the new test cases, it could be good to offset a little bit the source points so that they are not directly aligned with other source points. Otherwise, it is difficult to tell visually if a banked point is from one source point or the one directly aligned with it.

Here is what I did locally for a test but I wanted to see with you before committing it:

point_21 = openmc.stats.Point((1.6,0.5,0.5))
point_31 = openmc.stats.Point((2.6,0.5,0.5))
point_12 = openmc.stats.Point((0.5,0.5,1.6))

I like the idea of testing the duplicate cells strategy in test_duplicate_cells(). I think we can perform this test without running any calculations. I would suggest to remove this test from the regression tests and create a cpp unit test instead. In this test, we would just check that settings::ssw_cells is what we expect given a set of XML directives.

Let me know if you have any questions/comments and if you need help with the cpp unit test. Good examples of cpp unit tests are available in tests/cpp_unit_tests.