Implement surface flux tallies

[GuySten]

Description

This PR enable openmc to calculate surface flux tallies.

Fixes #2.

Theory

https://www.tandfonline.com/doi/pdf/10.13182/NSE09-72

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]

The tests does not pass because of #3743.

[GuySten]

This PR is ready for review.

[pshriwise]

Thanks for the PR @GuySten! I'm wondering what your thoughts are on providing the $\mu$ cutoff and ratio values on SurfaceFitlers themselves. As indicated in the paper linked in the description, different factors may be appropriate for different surfaces (for example, if the surface is interior or a boundary surface). Thus, it might make sense to allow these to be applied to SurfaceFitler's themselves. When scoring, I think it would be pretty clean to look up the SurfaceFitler object using Tally:get_filter and adjust the score as needed depending on the value of $|\mu|$ computed above the tally loop.

Given that the score requires a tally have a SurfaceFilter and that it isn't meaningful to have more than one SurfaceFilter on a tally (though interestingly we do currently allow it).

Let me know what you think!

[GuySten]

If I remember correctly there is a case when there are only CellFrom and Cell filters without surface filters.

I think if there is interest we can add in a subsequent PR attributes to the Surface objects themselves that will override the defaults in the setting object.

[pshriwise]

If I remember correctly there is a case when there are only CellFrom and Cell filters without surface filters.

I think if there is interest we can add in a subsequent PR attributes to the Surface objects themselves that will override the defaults in the setting object.

Ah, yes, right you are. Part of the motivation for my comment was that we avoid additional global settings, but in light of the CellFrom/Cell case you mentioned it seems the best approach.

[pshriwise]

Haven't forgotten about this. Will give it another look tomorrow

[GuySten]

@pshriwise, are you reviewing this PR?

[paulromano]

Thanks a lot for implementing this @GuySten! A few comments below that shouldn't be too difficult to address:

[GuySten]

I thought about another problem.
current has direction so it makes sense to tally partial directed currents from one cell to the other.
But flux has no direction so the usage of either CellFilter or CellFrom filter will tally only particles in one direction when we actually need to also tally particles in the other direction.

I will think about a way to fix that so particles in the other direction will also score.

EDIT:
I fixed this problem by using a virtual particle with current cell and last cell switched and using the of the two particles scores in surface flux score.

[GuySten]

@paulromano, I addressed all your concerns.

[paulromano]

@GuySten Thanks so much for your work on this feature. After your latest updates, I did some more refactoring to simplify the implementation, notably:

• Eliminated the need for CombFilterBinIter and the mirrored particle in score_surface_tally
• Made it so that SurfaceFilter is independent of the score; it always assigns a filter weight of 1.0 and the direction for current is now handled as part of score_surface_tally

Take a look and let me know if you're OK with the updates!

[GuySten]

My changes made that surface flux from cell1 to cell2 will be identical to surface flux from cell1 to cell2, both of them will have the meaning of flux averaged over the surface between cell1 and cell2.
After you removed CombFilterBinIter and the symmetic particle that is not the case anymore.
I think this should be fixed before merging.

[paulromano]

I understand the motivation behind wanting the flux from cell1 -> cell2 to be equivalent to the flux from cell2 -> cell1, but to me that behavior would be surprising and inconsistent with how the tally system in OpenMC works. With a few exceptions, filters are intended to limit the events that score to a tally. If a user adds a CellFromFilter, my expectation is that only particles originating "from that cell" should score to the tally. By allowing it to have the double meaning of also "cell to", it becomes confusing as to what the filter is really indicating. Another concern I have is that it creates dependencies between filters, which again makes it hard to reason about the overall behavior.

[GuySten]

As long as this decision is made knowingly I am okay with it.
If a user want the flux averaged over a surface between cell1 and cell2 he will still have the option to tally surface flux from cell1 to cell2 and surface flux from cell2 to cell1 and add them together.
So I am okay with the updates.