[Core] Add Voxel Mesh Generation Modeler #12297

jcotela · 2024-04-19T14:46:25Z

📝 Description

This PR adds the Voxel Mesh Generator Modeler developed at @KratosMultiphysics/altair to the Kratos core. The modeler uses a modular approach to combine different strategies for its various steps:

Generation of the key planes of the underlying regular grid
Coloring of sets of voxels within the grid based on different criteria
Mesh generation (which acts on all voxels of the same color) for tetrahedra, quadrilateral and their faces.
ModelPart operations, which are post-process steps that can be applied after meshing and make use of the meshing data (currently used to find contacts between different sets of generated entities)

The modeler is registered in KratosCore so that it can be used through the standard factory.

Also included in the PR are C++ and Python tests for multiple meshing configurations.

🆕 Changelog

Add VoxelMeshGeneratorModeler

philbucher

Great addition!

This is pretty large for reviewing, some initial comments/observations

kratos/tests/cpp_tests/modeler/test_key_plane_generation_with_refinement.cpp

philbucher · 2024-04-21T14:01:56Z

kratos/modeler/coloring/color_cells_with_inside_center.h

+                r_colors.AddGeometry(r_geometry, false);
+            }
+        } else{
+            KRATOS_ERROR << "The input_entities  " << parameters["input_entities"] << " is not supported. The supported input_entities are  elements and conditions" << std::endl;


missing geometries

updated the message

philbucher · 2024-04-21T14:02:38Z

kratos/modeler/coloring/color_cells_with_inside_center.h

+            for(auto& r_element : r_model_part.Elements()) {
+                Element::GeometryType& r_geometry = r_element.GetGeometry();
+                CheckGeometryType(r_geometry.GetGeometryType());
+                r_colors.AddGeometry(r_geometry, false);
+            }
+        } else if(input_entities == "conditions"){
+            for(auto& r_condition : r_model_part.Conditions()) {
+                Condition::GeometryType& r_geometry = r_condition.GetGeometry();
+                CheckGeometryType(r_geometry.GetGeometryType());
+                r_colors.AddGeometry(r_geometry, false);
+            }


conceptual question, should those still be allowed?
The overall goal is to move away from using elements and conditions in the input and only use geometries

I have nothing to say about the longer term plan, but I can tell you that we typically use this with a "traditional" mdpa file as input, so supporting elements is important for us at the moment.

philbucher · 2024-04-21T16:06:43Z

kratos/modeler/key_plane_generation/key_plane_generation_by_outer_shell.h

+        // Now we find the min and max value for the node coordinates
+        Vector min_v(3,std::numeric_limits<double>::max()), max_v(3, -std::numeric_limits<double>::max());
+        for(auto& node: GetInputModelPart().Nodes()){
+            double x = node.X(), y = node.Y(), z = node.Z();
+            min_v[0] = std::min(min_v[0], x); max_v[0] = std::max(max_v[0], x);
+            min_v[1] = std::min(min_v[1], y); max_v[1] = std::max(max_v[1], y);
+            min_v[2] = std::min(min_v[2], z); max_v[2] = std::max(max_v[2], z);
+        }


this can be very easily parallelized with the parallel utils

Agreed, but that requires a reduction to a bounding box, which we don't have at the moment. I would rather do that on a separate PR, with proper tests, rather than make this one bigger. It would also be a good candidate for a custom reduction in the sense of #12195, if we go ahead with that one.

philbucher · 2024-04-21T16:07:49Z

kratos/modeler/voxel_mesh_generator_modeler.cpp

+    Timer::Stop("EntityGeneration");
+}
+
+ModelPart& VoxelMeshGeneratorModeler::CreateAndGetModelPart(std::string const& FullName)


I am pretty sure the Model does the same nowadays
@sunethwarna can you confirm pls?

I am using the model capabilities now, but I'm leaving a helper function to check for existence of the new model part (as in that case the model method logs a warning)

philbucher · 2024-04-21T16:08:15Z

kratos/modeler/voxel_mesh_generator_modeler.cpp

+        return *p_current_model_part;
+}
+
+ModelPart& VoxelMeshGeneratorModeler::GetModelPart(std::string const& FullName)


same, the Model does exactly this

This function was not used, so I'm removing it.

kratos/modeler/voxel_mesh_generator_modeler.h

philbucher · 2024-04-21T16:08:59Z

kratos/modeler/voxel_mesh_generator_modeler.h

+    ///@name Serializer
+    ///@{
+
+    friend class Serializer;


I dont see this used, is it?

philbucher · 2024-04-21T16:09:51Z

kratos/sources/kratos_application.cpp

+    //Register Voxel Modeler modular components
+    RegisterVoxelMesherColoring();
+    RegisterVoxelMesherKeyPlaneGeneration();
+    RegisterVoxelMesherEntityGeneration();
+    RegisterVoxelMesherOperation();


Is this really necessary? Usually this is done generic, why does the VoxelMesher require different treatment?

I'm not sure if I understand you correctly but the only special treatment the VoxelMesher gets in terms of the registry is that it uses its own factory class. This is due to historical reasons (it started as a monolithic class and we eventually made the different components modular. The json input format that was already in use at that point is not compatible with what the standard Kratos factory expects, so we made our own factory class to parse the existing json)

Just to clarify, this Factory is for internal use within the modeler (it is used to instantiate the requested components on demand). The modeler itself is registered using the same approach as for other C++ modelers, with the usual kratos factory class.

loumalouomega

A lot of code could be moved to source files, I will take a look later

loumalouomega · 2024-04-22T12:22:23Z

I don't know how, but I broke CentOS after 3be5c90 after just changing the headers¿?

jcotela · 2024-05-08T16:08:39Z

I think that I found the issue with the Centos7 CI, it turns out that the way some array_1d were being initialized from zero_vector was triggering an error if BOOST_UBLAS_TYPE_CHECK is enabled (I believe that this is implied by the -Wignored-qualifiers compiler flag? That macro is not defined in normal FullDebug builds)

@loumalouomega please let the CI run for this change before merging master, to see if we fixed the issue.

roigcarlo

Mostly pedantic comments from my part, and some remarks about #pragma omp in some places, but in general 👍

kratos/modeler/internals/cartesian_mesh_colors.h

roigcarlo · 2024-05-13T09:50:09Z

kratos/modeler/key_plane_generation/key_plane_generation_with_refinement.cpp

+            keep_merging = false;
+            if(r_i_partition.size()>3){
+                // The first one and the last one cannot be removed or moved.
+                for (int i = 1;  i+1 < (int) r_i_partition.size()-1; ++i) { //This is to ensure we don't remove the last one nor the first one so i+1 is the end - 1


Suggested change

for (int i = 1; i+1 < (int) r_i_partition.size()-1; ++i) { //This is to ensure we don't remove the last one nor the first one so i+1 is the end - 1

for (int i = 1; i+1 < static_cast<int>(r_i_partition.size()-1); ++i) { //This is to ensure we don't remove the last one nor the first one so i+1 is the end - 1

(or std::size_t i if not going to parallelize)

Same with all the dynamic cast in the file

kratos/modeler/entity_generation/generate_tetrahedral_elements_with_cell_color.cpp

roigcarlo · 2024-05-13T09:56:50Z

kratos/modeler/internals/cartesian_mesh_colors.h

+        return I + J * mElementCenterCoordinates[0].size() + K * mElementCenterCoordinates[1].size() * mElementCenterCoordinates[0].size();
+      }
+     public:
+      CartesianMeshColors(): mTolerance(1e-12), mMinPoint(0.00, 0.00, 0.00), mMaxPoint(0.00, 0.00, 0.00){}


Why double .00 instead of 0.0?

Happens in several places.

roigcarlo · 2024-05-13T09:58:20Z

kratos/modeler/internals/cartesian_mesh_colors.h

+        mNodalCoordinates[1] = rNodalYCoordinates;
+        mNodalCoordinates[2] = rNodalZCoordinates;
+
+        for(int i = 0 ; i < 3 ; i++){


I would use std::size_t to keep it consistent.

Same with the other loops in the file.

roigcarlo · 2024-05-13T10:29:46Z

kratos/modeler/voxel_mesh_generator_modeler.cpp

+
+namespace Kratos
+{
+using SizeType = std::size_t;


std::size_t is being used directly in the file in most of the cases. Unless the implementation for some code is not dependent on the size I would just remove this.

kratos/modeler/voxel_mesh_generator_modeler.cpp

roigcarlo · 2024-05-13T10:34:44Z

kratos/modeler/voxel_mesh_generator_modeler.h

+    typedef std::size_t SizeType;
+    typedef std::size_t IndexType;


Not used (that I've seen)

roigcarlo · 2024-05-13T10:39:18Z

kratos/tests/cpp_tests/modeler/test_key_plane_generation_with_refinement.cpp

+{
+    using namespace Kratos;
+
+	WriteCubeSkinMeshMdpaFile();


Please remove file after test has finished (for all the tests).

kratos/modeler/operation/find_contacts_in_skin_model_part.cpp

Co-authored-by: Carlos Roig <roigcarlo@users.noreply.github.com>

loumalouomega · 2024-05-14T09:38:23Z

kratos/modeler/operation/find_contacts_in_skin_model_part.cpp

+    const int cell_color = parameters["cell_color"].GetInt();
+    KRATOS_INFO("Modeler") << "Finding contacts for " << r_skin_part.FullName()  << " model part" << std::endl;
+
+    block_for_each(r_skin_part.Conditions(),[&](Condition& rCondition) {


@roigcarlo @jcotela the critical is called here

…ysics/Kratos into feature/voxel-mesh-modeler

jcotela · 2024-05-14T15:22:07Z

I think we have addressed all review comments. Feel free to add some more.

jcotela · 2024-05-15T09:31:14Z

@roigcarlo I have removed the active pragma omp (replaced them by IndexPartitions) but I have left the ones that were commented out. I would like to keep them as a reminder of places were we disabled parallelism, since it might still be possible to make them parallel in the future. We will address those in a separate PR.

loumalouomega

I think this is ready to goI

roigcarlo · 2024-05-15T10:34:00Z

👍 from my side as well.

jcotela added 7 commits April 16, 2024 15:51

Adding utility function to add ordered nodes in batch to modelpart

6d8847a

Add Voxel mesh generator modeler

45bf136

Add cpp tests

220a28f

Adding python tests

b7cec84

Use new factory format

9fe8d30

Adding missing interface to Operation

0030a7a

Merge master

f11c9e5

jcotela requested a review from a team as a code owner April 19, 2024 14:46

loumalouomega changed the title ~~[core] Add Voxel Mesh Generation Modeler~~ [Core] Add Voxel Mesh Generation Modeler Apr 19, 2024

philbucher reviewed Apr 21, 2024

View reviewed changes

Fix headers

3be5c90

loumalouomega reviewed Apr 22, 2024

View reviewed changes

Merge branch 'master' into feature/voxel-mesh-modeler

d131e13

jcotela and others added 10 commits April 22, 2024 16:00

Fix non-deallocated pointer and debug output

4c0501f

Missing const

5515036

Merge master

bebd81c

Typo

5f1f338

Fix warning message

b5fc155

Remove unused function

c816e34

Use existing modeler capabilities to create new modelparts

9eba80f

Remove unused friend

d819776

Merge branch 'master' into feature/voxel-mesh-modeler

55ef57d

Fix runtime error if BOOST_UBLAS_TYPE_CHECK is enabled

8825367

loumalouomega and others added 5 commits May 9, 2024 11:23

Moving coloring to source files

cc71737

Fix unity compilation

9ecc213

Refactor source/header for entity generation

6f3ec98

More source/header refactor

5b3b33f

Defining destructors

82b371b

roigcarlo reviewed May 13, 2024

View reviewed changes

jcotela and others added 8 commits May 13, 2024 12:49

Removing unused code from old factory implementation

aaa9bd6

Use existing function instead of duplicating code

1e0269f

Apply suggestions from code review

5551164

Co-authored-by: Carlos Roig <roigcarlo@users.noreply.github.com>

simplify bool operation

de8d1f7

Co-authored-by: Carlos Roig <roigcarlo@users.noreply.github.com>

Removing output

0147cc9

Missing semicolon

cec8cd6

Delete auxiliary file

936d1ad

Suggestion

c676e88

loumalouomega reviewed May 14, 2024

View reviewed changes

jcotela added 6 commits May 14, 2024 11:48

style

47a7e55

Rewrite loop conditions to avoid cast to int

0e549f0

Merge branch 'feature/voxel-mesh-modeler' of github.com:KratosMultiph…

d188d0e

…ysics/Kratos into feature/voxel-mesh-modeler

Using std::size_t and parallel_utilities loops where possible

f85ebc1

Review suggestions in find contacts in skin

5492eb3

Remove unused types

5542da6

loumalouomega requested changes May 15, 2024

View reviewed changes

loumalouomega approved these changes May 15, 2024

View reviewed changes

jcotela merged commit 94a3e3b into master May 16, 2024
11 checks passed

jcotela deleted the feature/voxel-mesh-modeler branch May 16, 2024 09:23

jcotela mentioned this pull request May 16, 2024

Add missing KRATOS_API #12387

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Core] Add Voxel Mesh Generation Modeler #12297

[Core] Add Voxel Mesh Generation Modeler #12297

jcotela commented Apr 19, 2024

philbucher left a comment

philbucher Apr 21, 2024

jcotela May 7, 2024

philbucher Apr 21, 2024

jcotela Apr 22, 2024

philbucher Apr 21, 2024

jcotela May 7, 2024

philbucher Apr 21, 2024

jcotela May 7, 2024

philbucher Apr 21, 2024

jcotela May 7, 2024

philbucher Apr 21, 2024

jcotela May 7, 2024

philbucher Apr 21, 2024

jcotela Apr 22, 2024

jcotela Apr 22, 2024

loumalouomega left a comment

loumalouomega commented Apr 22, 2024

jcotela commented May 8, 2024

roigcarlo left a comment

roigcarlo May 13, 2024

roigcarlo May 13, 2024

roigcarlo May 13, 2024

roigcarlo May 13, 2024

roigcarlo May 13, 2024

roigcarlo May 13, 2024

loumalouomega May 14, 2024

jcotela commented May 14, 2024

jcotela commented May 15, 2024

loumalouomega left a comment

roigcarlo commented May 15, 2024

	for (int i = 1; i+1 < (int) r_i_partition.size()-1; ++i) { //This is to ensure we don't remove the last one nor the first one so i+1 is the end - 1
	for (int i = 1; i+1 < static_cast<int>(r_i_partition.size()-1); ++i) { //This is to ensure we don't remove the last one nor the first one so i+1 is the end - 1

[Core] Add Voxel Mesh Generation Modeler #12297

[Core] Add Voxel Mesh Generation Modeler #12297

Conversation

jcotela commented Apr 19, 2024

philbucher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loumalouomega left a comment

Choose a reason for hiding this comment

loumalouomega commented Apr 22, 2024

jcotela commented May 8, 2024

roigcarlo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jcotela commented May 14, 2024

jcotela commented May 15, 2024

loumalouomega left a comment

Choose a reason for hiding this comment

roigcarlo commented May 15, 2024