Implement generateCSG method for PinMeshGenerator #31782

this could be checking volumes? Or simplifying them and making sure a region is not empty? (intersection of +plane and -plane for example) ? or make a converter to OpenMC so the reviewer can plot them there?

modules/reactor/doc/content/source/meshgenerators/PinMeshGenerator.md

modules/reactor/include/meshgenerators/PinMeshGenerator.h

modules/reactor/test/tests/meshgenerators/pin_mesh_generator/tests

modules/reactor/src/meshgenerators/PinMeshGenerator.C

modules/reactor/test/tests/meshgenerators/pin_mesh_generator/gold/pin_hex_3d_csg.json

modules/reactor/src/meshgenerators/PinMeshGenerator.C

kkiesling · 2025-10-28T19:22:04Z

I think we should make some sort of "tester" for the CSG, to ensure it is valid

this could be checking volumes? Or simplifying them and making sure a region is not empty? (intersection of +plane and -plane for example) ? or make a converter to OpenMC so the reviewer can plot them there?

@GiudGiud Unfortunately, this is not necessarily within scope of the funded work that @shikhar413 and I currently doing. Yes, having some sort of visualization would be helpful as a developer, but that will also naturally come as the connection to specific codes are developed and each code has its native visualization tool that can be used. Additionally, Ideally tests are written in small enough units that the output can be manually inspected by developers/reviewers for correctness (which it seems you were able to do here, noting the mistake), and then they cover enough cases to be able to trust the larger complex models are correct.

Also, this CSG framework is not necessarily meant to check the validity of a CSG model, but instead makes sure it matches whatever input it was given. We are operating under the assumption that MOOSE has already validated the input as valid, and therefore if the CSG model is made to match that correctly, it is also valid. So in the example of an empty region, if that is what is created by the MOOSE input and valid there, then that is what would be created and considered valid for CSG as well.

Let us know if you want to discuss this in more detail offline/in a meeting.

moosebuild · 2025-10-28T21:13:42Z

Job Precheck, step Clang format on a7c8afe 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/31782/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 2acb415a170285fef37748f90025af38c3977b3c

modules/reactor/test/tests/meshgenerators/pin_mesh_generator/gold/pin_square_3d_csg.json

modules/reactor/src/meshgenerators/PinMeshGenerator.C

shikhar413 · 2025-10-31T20:10:37Z

@GiudGiud this should be good for a re-review now

…e downstream MG's

moosebuild · 2025-10-31T21:20:25Z

Job Test, step Results summary on 60f35e4 wanted to post the following:

Framework test summary

Compared against ee0740d in job civet.inl.gov/job/3349339.

Removed tests

Added tests

Test	Time (s)
`csg.geometry/csg_only_one_to_many_dependencies`	0.82

Run time changes

Modules test summary

Compared against ee0740d in job civet.inl.gov/job/3349339.

Removed tests

Added tests

Test	Time (s)
`reactor/test:meshgenerators/pin_mesh_generator.csg/2d_square`	0.84
`reactor/test:meshgenerators/pin_mesh_generator.csg/3d_hex`	0.81
`reactor/test:meshgenerators/pin_mesh_generator.csg/2d_hex_single_assembly`	0.80
`reactor/test:meshgenerators/pin_mesh_generator.csg/2d_square_single_assembly`	0.79
`reactor/test:meshgenerators/pin_mesh_generator.csg/2d_hex_homogenized`	0.79
`reactor/test:meshgenerators/pin_mesh_generator.csg/3d_hex_single_assembly`	0.78
`reactor/test:meshgenerators/pin_mesh_generator.csg/3d_hex_homogenized`	0.77
`reactor/test:meshgenerators/pin_mesh_generator.csg/3d_square`	0.77
`reactor/test:meshgenerators/pin_mesh_generator.csg/2d_hex`	0.76

Run time changes

GiudGiud · 2025-11-05T02:09:37Z

framework/doc/content/source/csg/CSGBase.md

-For example:
+These functions should be called from the constructor of the MeshGenerator, so that the MeshGenerator system can properly define the dependency tree of all mesh generators in the input file. The returned CSGBase pointers can be stored in a member variable and updated in the `generateCSG()` method in order to make any changes to the CSGBase object.
+
+For example, the following member variable stores the pointer to the CSGBase object that is generated by an input mesh geenerator:


Suggested change

For example, the following member variable stores the pointer to the CSGBase object that is generated by an input mesh geenerator:

For example, the following member variable stores the pointer to the CSGBase object that is generated by an input mesh generator:

framework/doc/content/source/csg/CSGBase.md

GiudGiud · 2025-11-05T02:12:14Z

framework/include/base/MeshGeneratorSystem.h


-  /// Holds the output CSGBase object for each mesh generator
-  std::map<std::string, std::unique_ptr<CSG::CSGBase>> _csg_base_output;
+  /// Holds the output CSGBase object for each mesh generator - including duplicates needed for downstream


Suggested change

/// Holds the output CSGBase object for each mesh generator - including duplicates needed for downstream

/// Holds the output CSGBase object for each mesh generator - including duplicates when needed by multiple downstream generators (key is MG name, value list is duplicates)

GiudGiud · 2025-11-05T02:14:26Z

framework/include/csg/CSGBase.h

+   * @brief Add a new universe to the universe list based on a universe reference.
+   * This method is called by the copy constructor of CSGBase
+   *
+   * @param cell reference to CSGCell that should be added to universe list


Suggested change

* @param cell reference to CSGCell that should be added to universe list

* @param univ reference to CSGUniverse that should be added to universe list

GiudGiud · 2025-11-05T02:15:40Z

framework/include/csg/CSGPlane.h

  // calculate the equivalent coeffients (aX + bY + cZ = d) from 3 points on a plane
  void coeffsFromPoints(const Point & p1, const Point & p2, const Point & p3);

+  // Normalize plane coefficients so that a^2 + b^2 + c^2 = 1


Suggested change

// Normalize plane coefficients so that a^2 + b^2 + c^2 = 1

/// Normalize plane coefficients so that a^2 + b^2 + c^2 = 1

// is for comments, /// is for docstring

GiudGiud · 2025-11-05T03:18:33Z

framework/src/csg/CSGBase.C

+  // Link all cells in other_base root universe to current root universe
+  for (auto & root_cell : other_base.getRootUniverse().getAllCells())
+  {
+    const auto & list_cell = _cell_list.getCell(root_cell.get().getName());


could be an efficiency issue to do everything by name but only time and profiling will tell (no action item now)

GiudGiud · 2025-11-05T03:24:03Z

framework/src/utils/CSGUtils.C

+{
+
+CSGRegion
+getInnerRegion(const std::vector<std::reference_wrapper<const CSGSurface>> & radial_surfaces,


Suggested change

getInnerRegion(const std::vector<std::reference_wrapper<const CSGSurface>> & radial_surfaces,

getInnerRegion(const std::vector<std::reference_wrapper<const CSGSurface>> & surfaces,

test/src/csg/NestedCellUniverseMeshGenerator.C

test/src/csg/TestOneToManyDependencyCombinerGenerator.C

test/src/csg/TestOneToManyDependencyMeshGenerator.C

shikhar413 requested review from GiudGiud, lindsayad, loganharbour and roystgnr as code owners October 24, 2025 17:04

shikhar413 added 2 commits October 24, 2025 13:12

Implement generateCSG method for 2D and 3D meshes defined using PinMe…

91b0fc5

…shGenerator, refs idaholab#31733

Add CSG unit tests for PinMeshGenerator

a9d2d8c

shikhar413 force-pushed the pin_mg_to_csg branch from 7872670 to d2f3a18 Compare October 24, 2025 17:18

shikhar413 force-pushed the pin_mg_to_csg branch from d2f3a18 to 762d24e Compare October 24, 2025 18:48

GiudGiud requested changes Oct 27, 2025

View reviewed changes

GiudGiud self-assigned this Oct 27, 2025

shikhar413 force-pushed the pin_mg_to_csg branch from 762d24e to a7c8afe Compare October 28, 2025 21:06

shikhar413 force-pushed the pin_mg_to_csg branch from a7c8afe to 7f225eb Compare October 28, 2025 21:26

GiudGiud reviewed Oct 29, 2025

View reviewed changes

modules/reactor/test/tests/meshgenerators/pin_mesh_generator/gold/pin_square_3d_csg.json Show resolved Hide resolved

GiudGiud reviewed Oct 29, 2025

View reviewed changes

shikhar413 force-pushed the pin_mg_to_csg branch from 7f225eb to 640ef1f Compare October 29, 2025 18:34

GiudGiud reviewed Oct 30, 2025

View reviewed changes

modules/reactor/src/meshgenerators/PinMeshGenerator.C Outdated Show resolved Hide resolved

shikhar413 force-pushed the pin_mg_to_csg branch 2 times, most recently from b45dbf8 to c173e79 Compare October 31, 2025 20:10

shikhar413 added 5 commits October 31, 2025 16:11

Update documentation for PinMeshGenerator

c9a88ab

Address comments by kkiesling

417d5f1

Address comments by GiudGiud

be4f91b

Move getInnerRegion to CSGUtils file

72abc59

Normalize plane coefficients so that a, b, and c define a unit vector

1cce78b

shikhar413 added 3 commits October 31, 2025 16:11

Refactor CSGBase to be able to handle single MG dependency on multipl…

3aaaecc

…e downstream MG's

Update failing tests and add new ones

9336c3a

Update documentation based on CSGBase refactoring

60f35e4

shikhar413 force-pushed the pin_mg_to_csg branch from c173e79 to 60f35e4 Compare October 31, 2025 20:12

GiudGiud reviewed Nov 5, 2025

View reviewed changes

	For example, the following member variable stores the pointer to the CSGBase object that is generated by an input mesh geenerator:
	For example, the following member variable stores the pointer to the CSGBase object that is generated by an input mesh generator:

	/// Holds the output CSGBase object for each mesh generator - including duplicates needed for downstream
	/// Holds the output CSGBase object for each mesh generator - including duplicates when needed by multiple downstream generators (key is MG name, value list is duplicates)

	* @param cell reference to CSGCell that should be added to universe list
	* @param univ reference to CSGUniverse that should be added to universe list

	// Normalize plane coefficients so that a^2 + b^2 + c^2 = 1
	/// Normalize plane coefficients so that a^2 + b^2 + c^2 = 1

	getInnerRegion(const std::vector<std::reference_wrapper<const CSGSurface>> & radial_surfaces,
	getInnerRegion(const std::vector<std::reference_wrapper<const CSGSurface>> & surfaces,

Implement generateCSG method for PinMeshGenerator #31782

Are you sure you want to change the base?

Implement generateCSG method for PinMeshGenerator #31782

Uh oh!

Conversation

shikhar413 commented Oct 24, 2025

Reason

Design

Impact

Uh oh!

shikhar413 commented Oct 24, 2025

Uh oh!

moosebuild commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

moosebuild commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Framework coverage

Modules coverage

Reactor

Full coverage reports

Uh oh!

shikhar413 commented Oct 25, 2025

Uh oh!

GiudGiud left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kkiesling commented Oct 28, 2025

Uh oh!

moosebuild commented Oct 28, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shikhar413 commented Oct 31, 2025

Uh oh!

moosebuild commented Oct 31, 2025

Framework test summary

Removed tests

Added tests

Run time changes

Modules test summary

Removed tests

Added tests

Run time changes

Uh oh!

GiudGiud Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

GiudGiud Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

GiudGiud Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

GiudGiud Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

GiudGiud Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

GiudGiud Nov 5, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

moosebuild commented Oct 24, 2025 •

edited

Loading

moosebuild commented Oct 24, 2025 •

edited

Loading