Set all tally estimators to collision in RR simulations

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.

[nuclearkevin]

Description

Obtaining results from a random ray simulation through the use of tallies in OpenMC relies of mapping each source region to a corresponding tally object. To facilitate this, source regions save the midpoint of the first ray that passes through them, which is then used to query tallies and find every tally object that maps spatially to the source region. In the vast majority of cases this works perfectly, however we run into issues when a tally is set to use a tracklength estimator. The filter matches run bins_crossed() using a position (r()) and previous position (r_last()) that are the same, which results in zero-lengths being returned. In most filters this still counts as a bin being hit, but in some cases (MOAB/XDG unstructured mesh tallies, and very rarely tallies with no filters applied) this will not be registered which leads to source regions missing tally objects.

This PR fixes this bug by silently setting all tallies to collision estimators before running a random ray simulation to ensure filter matches are obtained with r(). This has no impact on results as the random ray solver does not require the concept of estimators.

Checklist

• I have performed a self-review of my own code
• I have run clang-format (version 18) 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]

This feels a bit hacky.
Maybe it will be better to do something special in another place if we are in random ray mode.
Regardless, IMO you should add a test that now passes and used to fail without this change.

[pshriwise]

If we feel that the current behavior of the StructuredMesh::bins_crossed is correct (certainly is from the perspective of the random ray solver), then another option here would be to update the UnstructuredMesh::bins_crossed methods to call get_bin if the start and end position of a track are coincident and return that sole bin for consistency with the other mesh implementations.