-
Notifications
You must be signed in to change notification settings - Fork 1.2k
CSG 2D Lattice Support #31993
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
base: next
Are you sure you want to change the base?
CSG 2D Lattice Support #31993
Conversation
…attice accordingly
… virtual function
…(needs more tests)
…ated - tests need to be updated
…il populated - tests need to be updated" This reverts commit 3f6e101adea23476b7e093ffc42914a1f59122ae.
… doesn't change anything that needs to be managed by base
…e names and conversion of any data type
Note: updates and confirmation/review for correctness are required; this assumes a (x,y) type of grid indexing scheme, but that may need to be (ring, index) instead. Or conversion methods between the two are needed. Tests are still needed too.
…ds instead of getDimensions directly
…t with a map instead
…es obsolete; only set pitch
| @@ -1 +1 @@ | |||
| Subproject commit 7a7e52d8ffbb7223b46768ff88885c8c52f71855 | |||
| Subproject commit ed12daf37cc1919b018483d879776ef002a94595 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution! This contains a submodule update
| @@ -1 +1 @@ | |||
| Subproject commit 7a7e52d8ffbb7223b46768ff88885c8c52f71855 | |||
| Subproject commit ed12daf37cc1919b018483d879776ef002a94595 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution! This contains a submodule update
|
Job Precheck, step Clang format on d0f6835 wanted to post the following: Your code requires style changes. A patch was auto generated and copied here
Alternatively, with your repository up to date and in the top level of your repository:
|
| @@ -1 +1 @@ | |||
| Subproject commit 7a7e52d8ffbb7223b46768ff88885c8c52f71855 | |||
| Subproject commit ed12daf37cc1919b018483d879776ef002a94595 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution! This contains a submodule update
d0f6835 to
b34fc06
Compare
| @@ -1 +1 @@ | |||
| Subproject commit 7a7e52d8ffbb7223b46768ff88885c8c52f71855 | |||
| Subproject commit ed12daf37cc1919b018483d879776ef002a94595 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution! This contains a submodule update
| @@ -1 +1 @@ | |||
| Subproject commit 7a7e52d8ffbb7223b46768ff88885c8c52f71855 | |||
| Subproject commit ed12daf37cc1919b018483d879776ef002a94595 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution! This contains a submodule update
| @@ -1 +1 @@ | |||
| Subproject commit 7a7e52d8ffbb7223b46768ff88885c8c52f71855 | |||
| Subproject commit ed12daf37cc1919b018483d879776ef002a94595 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution! This contains a submodule update
|
This looks like it will pass CI at this point, so it should be ready for review @GiudGiud |
|
@eshemon FYI |
|
Job Test, step Results summary on f0a2d2f wanted to post the following: Framework test summaryCompared against b85414d in job civet.inl.gov/job/3409862. Added tests
Modules test summaryCompared against b85414d in job civet.inl.gov/job/3409862. No change |
|
Job Documentation, step Docs: sync website on f0a2d2f wanted to post the following: View the site here This comment will be updated on new commits. |
|
Job Coverage, step Generate coverage on f0a2d2f wanted to post the following: Framework coverage
Modules coverageCoverage did not change Full coverage reportsReports
This comment will be updated on new commits. |
||||||||||||||||||||||||||
| A `CSGLattice` is defined as a patterned arrangement of [`CSGUniverse`](#universes) objects and an "outer" to fill the space around lattice elements. | ||
| The `CSGBase` class provides support for two types of 2D lattice types: Cartesian and regular hexagonal. | ||
| It is assumed that these two types are in the $x-y$ plane (having a $+z$ norm). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| It is assumed that these two types are in the $x-y$ plane (having a $+z$ norm). | |
| It is assumed that these two types are in the $x-y$ plane (having a $+z$ normal). |
| !alert! note title=2D vs. 3D Lattices | ||
| The `CSGBase` class supports only the creation of 2D lattices. A "3D" lattice can be created by filling multiple 3D cells with 2D lattices and arranging them in layers to form the desired 3D structure. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| The `CSGBase` class supports only the creation of 2D lattices. A "3D" lattice can be created by filling multiple 3D cells with 2D lattices and arranging them in layers to form the desired 3D structure. | |
| The `CSGBase` class supports only the creation of 2D lattices. A "3D" lattice can be created by filling multiple 3D cells with 2D lattices and arranging them in layers to form the desired 3D structure, or by modifying the cells into a 2D lattice to using extrusion in the third dimension. |
reactor module always does extrusion at the last step. We ll need to be able to turn these 2D cells into 3D cells then.
| * @param data std::any object to convert | ||
| * @return nlohmann::json representation of the data | ||
| */ | ||
| nlohmann::json anyToJson(const std::any & data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make a new file "JSONUtils.h" we wont find this new cool routine otherwise
| The `CSGLattice` objects can be accessed or updated with the following methods from `CSGBase`: | ||
| - `setLatticeUniverses`: sets the vector of vectors of `CSGUniverse` objects as the lattice layout. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is kind of assuming a regular layout. The FlexiblePattern (widely used) would probably not work here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it would just work with a single vector passed as a 2d vector
| - `resetLatticeOuter`: resets the outer fill to void for the lattice. | ||
| - `renameLattice`: change the name of the `CSGLattice` object. | ||
| - `getAllLattices`: retrieve a list of const references to each `CSGLattice` object in the `CSGBase` instance. | ||
| - `getLatticeByName`: retrieve a const reference to the lattice object of the specified name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as mentioned, this is a little different that the usual source documentation, as it overlaps with the doxygen. I generally would not recommend listing routines, though I wont ask that you change this either
| auto name = lattice.getName(); | ||
| if (!checkLatticeInBase(lattice)) | ||
| mooseError("Cannot set universe at index for lattice " + name + | ||
| ". Lattice is different from the lattice of the same name in the " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do we know it has the same name
| const Real _inner_rad; | ||
| /// radius of outer sphere surface | ||
| const Real _outer_rad; | ||
| /// radius of lattice sphere surface |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lattices dont have regions?
| // determine fill: either from input fill mesh generator or default material | ||
| if (_has_fill) | ||
| { | ||
| // join the fill CSGBase into the current CSGBase & use the lattice as the fille |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // join the fill CSGBase into the current CSGBase & use the lattice as the fille | |
| // join the fill CSGBase into the current CSGBase & use the lattice as the fill |
| params.addRequiredParam<std::vector<std::vector<unsigned int>>>( | ||
| "pattern", | ||
| "A double-indexed array starting with the upper-left corner where the index" | ||
| "represents the layout of inputs in the lattice."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "represents the layout of inputs in the lattice."); | |
| "represents the index of the mesh/CSG generator in the 'inputs' vector"); |
| const Real pitch, | ||
| std::vector<std::vector<std::reference_wrapper<const CSGUniverse>>> universes) | ||
| { | ||
| return addLattice(std::make_unique<CSGCartesianLattice>(name, pitch, universes)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could just do
template type T
addLattice
{
return addLattice(std::make_unique<T>(...);
}
closes #31889 , refs #30860
co-author @shikhar413
Reason
CSG Lattice support is needed for implementing the generateCSG method for rest of the RGMB mesh generators and other patterned MGs.
Design
Here are a few key design choices not previously discussed + some related questions (in bold):
Hexagonal and Cartesian lattices are directly supported by the CSGBase class, but any other type of custom 2D lattices could be defined by deriving from the CSGLattice class and added to the CSGBase instance (analogous to how CSGSurfaces are handled). The difference between the lattice and surface implementation for this is that I provide multiple methods directly in CSGBase for creating either a CSGHexagonalLattice or CSGCartesianLattice, rather than requiring that those types be created separately and then added to the base.
Per discussion on slack, I decided to implement the lattice "outer" region (ie the space between lattice elements that is undefined by a fill universe) to be either a CSG material (string ID) or a Universe.
For all the lattice type-specific information, I created a method called
getAttributeswhich provides all the "extra" info that isn't inherently known from the universe map (or that is convenient to have easily on hand), iepitchornrings. Because the data type of these attributes can be anything technically, I set it up to use thestd::anytype. However, if accessing this data, it requires specific casting to the expected data typestd::any_cast. In particular, the JSON dictionary doesn't like this data type, so I created the extra helper method in the CSGUtils file to convert these data types. This data type conversion isn't really CSG-specific though, so is there a different place in MOOSE that this method should live? Or, if this isn't the desired approach to handling the different types of data, thegetAttributesmethod could just be setup to create a JSON object instead, and this helper method would not be needed. However, depending on the decision on 6 below, we may want to leave thegetAttributesas it is now so that this sort of information can be accessed outside of JSON file creation.For hexagonal lattices, I implemented some methods to help convert between the (row, column) form and (ring, position) form. Per discussion on Slack, it was recommended that these methods be located in the HexagonalLatticeUtils file. However, that file appears to be in the Reactor module, and not the framework. Does it make sense for the framework to rely on a utils file in a module like that? I didn't move the methods due to this, but let me know if you think that is where they should live. Alternatively, there is the CSGUtils file now where I could move these to.
Because of the existence of the aforementioned extra methods in the CSGHexagonalLattice class, in order for those methods to be available after a lattice object is created via CSGBase, those
createXLatticemethods in CSGBase have to return the specific lattice type, rather than simply CSGLattice. I set this up to use dynamic casting to handle this. But this also required updating thegetLatticeByNamemethod to use a template so that the correct type is returned and these methods are accessible. The templated method is setup such that it will default to CSGLattice type if no type is provided, thus releasing the burden of having to do this a bit. Shikhar and I went back and forth on this a bit, and decided to wait for input on the review about whether we should use this template and specify the specific lattice return types, or if we should use the CSGLattice only. If we move the extra CSGHexagonalLattice methods to a utils file, then this becomes less necessary and we can set the methods up to just return CSGLattice in all cases. Methods likegetNRowsorgetPitchon these two lattice classes also wouldn't be available, but those values can also be retrieved viagetAttributes(though this requires use ofany_castto correctly gather the value, as noted above). So the question is: does it make sense to leave this as it is with the dynamic casting, or should we use CSGLattice only?Impact
Adds support for CSGLattices to the CSGBase class so that the generateCSG methods can be completed for various MGs relying on lattice forms.