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 Generator infrastructure changes to support --csg-only command line option #29102

Draft
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

shikhar413
Copy link
Contributor

refs #29095

@shikhar413 shikhar413 changed the title Csg only Mesh Generator infrastructure changes to support --csg-only command line option Nov 19, 2024
@shikhar413 shikhar413 force-pushed the csg_only branch 2 times, most recently from dd72729 to 3ee8f97 Compare November 19, 2024 20:38
@GiudGiud GiudGiud self-assigned this Nov 19, 2024
@moosebuild
Copy link
Contributor

moosebuild commented Nov 19, 2024

Job Documentation, step Docs: sync website on 2a243e6 wanted to post the following:

View the site here

This comment will be updated on new commits.

@shikhar413
Copy link
Contributor Author

@kkiesling @eshemon FYI

@shikhar413
Copy link
Contributor Author

shikhar413 commented Nov 19, 2024

@GiudGiud @loganharbour this is a preliminary PR that defines the --csg-only command line option for triggering CSG-based mesh generation. We (Kalin and I) figured this would be the best way to submit an intermediate PR. Trying to add the functionality for defining a CSGBase class object is not in this PR because it would become too large to review, so I've left some TODO's of areas that need to be updated. We've tried to make use of the work from last year's FY that invokes data driven generation for this work as well, which we would like to leverage directly, but we can schedule a separate meeting to discuss this code design

@moosebuild
Copy link
Contributor

Job Coverage, step Generate coverage on 2a243e6 wanted to post the following:

Framework coverage

37b999 #29102 2a243e
Total Total +/- New
Rate 85.13% 85.13% +0.00% 92.42%
Hits 107288 107345 +57 61
Misses 18742 18747 +5 5

Diff coverage report

Full coverage report

Modules coverage

Coverage did not change

Full coverage reports

Reports

This comment will be updated on new commits.

@loganharbour
Copy link
Member

I'm not sure that it makes sense to add this yet without the actual storage of the CSG representation? Has that component been discussed?

@shikhar413
Copy link
Contributor Author

Thought I’d push this up first to get your thoughts on whether this could be merged first before creating the data type for storage of CSG representation, since adding that data type will be quite a large number of changes as well. That portion has not been completed yet but if you’d like that part to be included in this PR as well we can treat this PR as a draft and you guys can review the code design so far. Let us know what makes most sense in terms of code review

@shikhar413
Copy link
Contributor Author

Just an update on this. I think I've figured out a way to generate a CSG mesh from any arbitrary Exodus file that is imported through FileMeshGenerator, the intermediate goal for this PR could then be to demonstrate --csg-only option + generateCSG works for FileMeshGenerator. Would that be sufficient for now? This way we can define the CSGBase object that is needed to get this running and go from there. Just as an example of what I have so far:

image

@GiudGiud
Copy link
Contributor

GiudGiud commented Nov 22, 2024

Adding CSG output for a mesh generator would be great to demonstrate this CSG-only mode.
FMG is the more complex one but also one of the more useful so feel free to do that one if you have a good lead!

what is your target output format? XML?
how are you conserving subdomains of elements in the mesh? Using universes / cells containing cells?

@shikhar413
Copy link
Contributor Author

what is your target output format? XML?
how are you conserving subdomains of elements in the mesh? Using universes / cells containing cells?

right now targeting json output format so as not to favor one MC code over another. Then the downstream code (Cardinal / Shift etc) can have an in memory hook to convert this json data to whatever format they need

@GiudGiud
Copy link
Contributor

Do you expect you'll output some explanation of the CSG through comments? This would make json a problem.

@shikhar413
Copy link
Contributor Author

Do you expect you'll output some explanation of the CSG through comments? This would make json a problem.

Wasn't planning to. Right now I'm thinking of outputting the info about the root universe, the cells associated with this universe, and the surfaces used to define those cells. Can be expanded accordingly one we start to support nested universes, lattices etc. Let me know if you think outputting to JSON has any specific limitations w.r.t CSG representation

@GiudGiud
Copy link
Contributor

GiudGiud commented Nov 22, 2024

I dont know enough. I just asked Paul on the crpg slack let's see what he thinks on formats.
I know the latest MC project chose YAML over XML and JSON and I think it's because it's nicer

@shikhar413
Copy link
Contributor Author

shikhar413 commented Nov 22, 2024

I’m honestly not too concerned with what the output data type is going to be here, as long as it contains the data that is needed to generate the CSG geometry. Ultimately I see this more of a debugging tool to make sure CSG-only is working as expected, and whatever format we choose we’re going to have to modify it to fit whatever the downstream MC code can read. We also might want to discuss whether we are creating a geometry specification file or a geometry input file (e.g. for OpenMC an XML file or a Python file?). Either can be possible

@GiudGiud
Copy link
Contributor

Ultimately I see this more of a debugging tool to make sure CSG-only is working as expected

Yes debugging and testing as well. I think that will be the main way of testing the CSG generation in MOOSE, which is why I'm happy to see this capability come in quite early.

We also might want to discuss whether we are creating a geometry specification file or a geometry input file (e.g. for OpenMC an XML file or a Python file?

This would be done in the OpenMC repo right? With a MOOSE-JSON-CSV reader/converter python script?

@shikhar413
Copy link
Contributor Author

This would be done in the OpenMC repo right? With a MOOSE-JSON-CSV reader/converter python script?

Yup, or probably Cardinal since I don't think OpenMC directly interfaces with finite element meshes. Will have to coordinate with the external stakeholders for MC codes to figure this step out, but for now we can focus on the FEM-->CSG JSON step (or another output format if there is a strong reason to do so)

@shikhar413 shikhar413 marked this pull request as draft November 22, 2024 19:01
@shikhar413
Copy link
Contributor Author

The other thing to also note is that if we are coupling with a C++ code we can ignore the output data type altogether and generate the final CSG mesh from the internal representation we create in MOOSE

Copy link
Contributor

@GiudGiud GiudGiud left a comment

Choose a reason for hiding this comment

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

quick comments because I was curious. Looking forward to seeing how you make the CSGBase work


#include "Action.h"

class CSGOnlyAction : public Action
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
class CSGOnlyAction : public Action
/**
* Outputs the Constructive Solid Geometry to file then exits
*/
class CSGOnlyAction : public Action

@@ -195,6 +220,11 @@ class MeshGenerator : public MooseObject, public MeshMetaDataInterface
*/
virtual void generateData();

/**
* Generate the CSG mesh
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Generate the CSG mesh
* Generate the CSG representation of the mesh (or meshing operation) created by this mesh generator

params.addCommandLineParam<std::string>(
"csg_only",
"--csg-only",
"Setup and Output the input mesh in CSG format only (Default: \"<input_file_name>_in.e\")");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Setup and Output the input mesh in CSG format only (Default: \"<input_file_name>_in.e\")");
"Setup and Output a constructive solid geometry (CSG) representation of the input mesh (Default: \"<input_file_name>_in.json\")");

void
MeshGenerator::generateInternalCSG()
{
mooseAssert(isDataOnly(), "Trying to use csg-only mode while not in data-driven mode");
Copy link
Contributor

Choose a reason for hiding this comment

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

?

@shikhar413
Copy link
Contributor Author

@GiudGiud @loganharbour just pushed where I'm at with integrating --csg-only with FileMeshGenerator. Everything looks like it's working as expected, just a few minor things left such as adding docstrings, documentation, and some more unit tests, as well as updating the logic for handling CSGBase objects in the CSGMeshOnlyAction. The CSGBase class is defined now, and I have a function CSGBase::generateOutput() which defines the OpenMC Python API input from Exodus mesh like so:

surf0 = openmc.Plane(name='surf0', a=-0.57735, b=1, c=0, d=0)
surf1 = openmc.Plane(name='surf1', a=-0.57735, b=-1, c=0, d=-1.1547, boundary_type='vacuum')
surf2 = openmc.Plane(name='surf2', a=1.1547, b=0, c=0, d=0)
surf3 = openmc.Plane(name='surf3', a=0.57735, b=1, c=-8.5876e-18, d=0)
surf4 = openmc.Plane(name='surf4', a=-1.1547, b=-2.22045e-16, c=-1.28198e-16, d=-1.1547, boundary_type='vacuum')
surf5 = openmc.Plane(name='surf5', a=1.1547, b=1.4141e-16, c=-1.40149e-33, d=0)
surf6 = openmc.Plane(name='surf6', a=-0.57735, b=1, c=8.5876e-18, d=-1.1547, boundary_type='vacuum')
surf7 = openmc.Plane(name='surf7', a=0.57735, b=-1, c=-8.5876e-18, d=0)
surf8 = openmc.Plane(name='surf8', a=0.57735, b=1, c=-2.34618e-17, d=-1.1547, boundary_type='vacuum')
surf9 = openmc.Plane(name='surf9', a=-0.57735, b=-1, c=-2.57628e-17, d=-0)
surf10 = openmc.Plane(name='surf10', a=1.1547, b=8.88178e-16, c=0, d=-1.1547, boundary_type='vacuum')
surf11 = openmc.Plane(name='surf11', a=0.57735, b=-1, c=2.57628e-17, d=-1.1547, boundary_type='vacuum')
void_cell = openmc.Cell(name='void_cell', fill=None, region=-surf1 & -surf4 & -surf6 & -surf8 & -surf10 & -surf11)
cell0 = openmc.Cell(name='cell0', fill=hexagon, region=+surf0 & +surf1 & +surf2)
cell1 = openmc.Cell(name='cell1', fill=hexagon, region=-surf0 & +surf3 & +surf4)
cell5 = openmc.Cell(name='cell5', fill=hexagon, region=-surf2 & -surf9 & +surf11)
cell2 = openmc.Cell(name='cell2', fill=hexagon, region=-surf3 & +surf5 & +surf6)
cell3 = openmc.Cell(name='cell3', fill=hexagon, region=-surf5 & +surf7 & +surf8)
cell4 = openmc.Cell(name='cell4', fill=hexagon, region=-surf7 & +surf9 & +surf10)
geometry = openmc.Geometry([void_cell, cell0, cell1, cell5, cell2, cell3, cell4])

will need to update this to generate a JSON file instead. I'll let you know when the PR is ready to remove from being in draft stage

@moosebuild
Copy link
Contributor

Job Precheck, step Clang format on 64f0696 wanted to post the following:

Your code requires style changes.

A patch was auto generated and copied here
You can directly apply the patch by running, in the top level of your repository:

curl -s https://mooseframework.inl.gov/docs/PRs/29102/clang_format/style.patch | git apply -v

Alternatively, with your repository up to date and in the top level of your repository:

git clang-format 9446d5fc904cc635205b563219f91889cd5a678c

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