Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mesh Source Class #2759

Merged
merged 73 commits into from
Dec 2, 2023
Merged

Conversation

pshriwise
Copy link
Contributor

Description

This adds the ability to specify an IndependentSource term for each element in a mesh, allowing for independent angle, energy, and time source distributions and unique source particle types in each element. The domain rejection feature added by @paulromano in #2235 can be used as well to ensure that particles are sourced within the specified domain(s) in the element. The MeshSource's strength is the combination of the strengths of sources applied to the mesh, and elements will be sampled based on their relative strength in the same way the MeshSpatial distribution handles this capability.

Quick note on choosing between a source that uses the MeshSpatial distribution and a MeshSource source.

For uniform source properties in each element (angle, energy, time, particle), use an IndependentSource with a MeshSpatial.

For unique energy, angle, time, particle distributions in each element, use a MeshSource and IndependentSource objects with distributions defined for each element as desired.

Fixes # (issue)

https://github.com/orgs/openmc-dev/projects/1/views/1?filterQuery=Mesh&pane=issue&itemId=1459193

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)

@pshriwise
Copy link
Contributor Author

Now that we can apply an arbitrary source type in the mesh elements, should I attempt to add some tests for source types other than IndependentSource?

@paulromano
Copy link
Contributor

@pshriwise Yes, not necessary to truly test the functionality of having some weird source on a mesh element but more so to make sure that there are no implicit assumptions about it being an independent source that are still hanging around.

@gonuke
Copy link
Contributor

gonuke commented Nov 13, 2023

This looks like a good addition for many general cases. I'd like to draw attention to a special case that falls in-between this solution and the previous case: mesh-based energy-dependent sources that share an energy group structure across all voxels/mesh elements. When this is true, the entire 4-D distribution can be setup with a single alias sampling scheme to accelerate the sampling.

It makes me wonder whether, in general, there is a desire to identify when the voxel-dependent distributions are discrete, so that it allows us to form a single discrete distribution across all of phase space?

@pshriwise
Copy link
Contributor Author

This looks like a good addition for many general cases. I'd like to draw attention to a special case that falls in-between this solution and the previous case: mesh-based energy-dependent sources that share an energy group structure across all voxels/mesh elements. When this is true, the entire 4-D distribution can be setup with a single alias sampling scheme to accelerate the sampling.

It makes me wonder whether, in general, there is a desire to identify when the voxel-dependent distributions are discrete, so that it allows us to form a single discrete distribution across all of phase space?

Good point, @gonuke. Somewhat connected to this thought -- I think one thing that will help here is the ability for sources to share distributions in memory, so if multiple IndependentSource objects have the same energy distribution, it won't be duplicated in memory. They currently are and that's flagged as an issue for scaling MeshSource's to large domains. This would help considerably us detect if sources are using the same distribution. (Just added a note to track this here)

As for detecting whether or not voxel-dependent distributions are discrete, we're currently allowing each mesh element to contain an arbitrary source type. Determining distribution equivalence for different FileSource's or CompiledSource's wouldn't be very easy, but for the set of mesh elements containing IndependentSource's we could figure this out. Still, if we construct alias tables to sample both the mesh element and energy distribution each operation is O(1). So while it would potentially be convenient to put the mesh and energy distributions in the same table for that use case, I'm not sure it's worth the added complexity that would come with the detection of identical source distributions, particularly when they may only account for part of the mesh. Am I missing a bigger motivation for this use case or misunderstanding the gains in sampling efficiency?

@gonuke
Copy link
Contributor

gonuke commented Nov 20, 2023

First, this wasn't meant to be a blocking comment.

Second, I think my use case makes more sense as an alternative input scheme in a different PR. Rather than detect the same distributions, allow users to define a source in this way for convenience.

Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pshriwise I made a few cosmetic updates here and there on your branch and think it's good to go now!

@paulromano paulromano enabled auto-merge (squash) December 2, 2023 15:43
@paulromano paulromano merged commit e0d0381 into openmc-dev:develop Dec 2, 2023
18 checks passed
church89 pushed a commit to openmsr/openmc that referenced this pull request Jul 18, 2024
Co-authored-by: Paul Romano <paul.k.romano@gmail.com>
Co-authored-by: Jonathan Shimwell <drshimwell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants