Add Pro/E input to intersection shaping driver by bmhan12 · Pull Request #1111 · llnl/axom

bmhan12 · 2023-06-06T18:08:24Z

This PR:

Adds Pro/E format as an input to the intersection shaping driver
Updates data submodule with klee and Pro/E files to test the new format in the quest_intersection_shaper unit tests:

Related to llnl/axom_data#16

Pro/E Case 1:

Pro/E Case 2:

kennyweiss

Thanks @bmhan12!

Please don't forget to update the RELEASE-NOTES.

kennyweiss · 2023-06-06T18:44:34Z

+    for(int i = 0; i < m_num_vertices; i++)
+    {
+      for(int j = i + 1; j < m_num_vertices; j++)
+      {
+        // operator= for Point does not want to play nice...
+        if(m_vertices[i][0] == m_vertices[j][0] &&
+           m_vertices[i][1] == m_vertices[j][1] &&
+           m_vertices[i][2] == m_vertices[j][2])
+        {
+          return false;
+        }
+      }
+    }


@bmhan12 -- do we always want to call a Polyhedron invalid when it has duplicated vertices?
There are some cases that I can think of where this might be beneficial.

Also, should we be checking for exact duplicates? or should we be supplying a tolerance and checking that the distance b/w vertices is not nearly zero subject to that tolerance?

Suggestion: I think it would be clearer to move the highlighted code to a new function hasDuplicatedVertices(double EPS), and consider calling it explicitly, when necessary.

do we always want to call a Polyhedron invalid when it has duplicated vertices?

Admittedly, I'm not sure.
I did find this case needs to be handled for the Polyhedron::volume() function. It resulted in an infinite loop condition.
I've added a new test in primal_clip.cpp for this case in particular.

Also, should we be checking for exact duplicates? or should we be supplying a tolerance and checking that the distance b/w vertices is not nearly zero subject to that tolerance?

👍 . Agreed, a tolerance value would be good.

Suggestion: I think it would be clearer to move the highlighted code to a new function hasDuplicatedVertices(double EPS), and consider calling it explicitly, when necessary.

I can separate this snippet into the function, and have it just in the Polyhedron::volume() function for now where I was originally seeing the problem.

kennyweiss · 2023-06-06T19:00:32Z

+    // Initialize tetrahedra
+    axom::Array<IndexType> nodeIds(4);
+    axom::Array<Point3D> pts(4);

-    const auto& shapeName = shape.getName();
-    AXOM_UNUSED_VAR(shapeDimension);
-    AXOM_UNUSED_VAR(shapeName);
+    for(int i = 0; i < m_tetcount; i++)
+    {
+      m_surfaceMesh->getCellNodeIDs(i, nodeIds.data());

-    SLIC_INFO(axom::fmt::format("Current shape is {}", shapeName));
+      m_surfaceMesh->getNode(nodeIds[0], pts[0].data());
+      m_surfaceMesh->getNode(nodeIds[1], pts[1].data());
+      m_surfaceMesh->getNode(nodeIds[2], pts[2].data());
+      m_surfaceMesh->getNode(nodeIds[3], pts[3].data());

+      m_tets[i] = TetrahedronType({pts[0], pts[1], pts[2], pts[3]});
+    }


Is there a way to do this w/in a kernel in the ExecSpace too?

Is there a way to do this w/in a kernel in the ExecSpace too?

I did not explore it in this PR, as a lot of these calls are to mint::UnstructuredMesh functions which are not host/device annotated yet.
The functions also override virtual mint::Mesh functions, so that might complicate things.

kennyweiss · 2023-06-06T19:11:29Z

@@ -882,11 +1005,12 @@ class IntersectionShaper : public Shaper
    axom::deallocate(m_hexes);


I'm a bit confused about when we use allocate/deallocate and when we use StackArray for variables/arrays.
Could you please clarify?

(Perhaps that's why I have so many comments about it)

I'm a bit confused about when we use allocate/deallocate and when we use StackArray for variables/arrays.
Could you please clarify?

The StackArray's that are being created here are for using the host-device decorated ArrayView constructor:https://github.com/LLNL/axom/blob/31ae2e4787a602d9a7b44eea76c3130a30c8ce3d/src/axom/core/ArrayView.hpp#L76-L87

The allocate/deallocate is for memory that will be used inside kernels.

I need to do a pass through these to change them to use axom::Array and axom::ArrayView.

agcapps

Thanks, Brian!

rhornung67

Thanks @bmhan12 A few comments for consideration related to documentation and implementation consistency

…est case for clipping

… the driver level

…ix wrong count

…e duplicate logic in driver

…to use Array to accomodate changes

bmhan12 · 2023-06-13T16:35:32Z

The conversion of pointer allocate/deallocate memory to axom:Array / axom::ArrayView was mostly straightforward.

Did encounter an error with HIP trying to use axom::Array with a RAJA::ReduceSum for the size:
https://github.com/LLNL/axom/blob/43df7b65cad3c0a1e625550458ffaf9e137ffebe/src/axom/quest/IntersectionShaper.hpp#L729-L755

Not seeing the problem with CUDA.

This case will be added to a running list of HIP issues in #787 .

bmhan12 added Quest Issues related to Axom's 'quest' component Primal Issues related to Axom's 'primal component labels Jun 6, 2023

bmhan12 requested review from BradWhitlock, agcapps, kennyweiss and publixsubfan June 6, 2023 18:08

bmhan12 mentioned this pull request Jun 6, 2023

Pro/E Unit Test Files llnl/axom_data#16

Merged

kennyweiss approved these changes Jun 6, 2023

View reviewed changes

agcapps approved these changes Jun 6, 2023

View reviewed changes

rhornung67 approved these changes Jun 6, 2023

View reviewed changes

bmhan12 added 20 commits June 12, 2023 15:07

Add read_pro_e_mesh() to QuestHelpers

9568fb8

Test read_pro_e_mesh() usage --not sure if I will keep this?

e534268

Update submodule with examples

6b10c22

Add Pro/E option to loadShape()

81ff869

WIP - separate out Pro/E and c2c workflow

8e832c4

sanity Pro/E mesh output

fc283fb

Add compute bounding box overload for tetrahedron

daafa84

WIP - setup tet mesh

253d72d

Post merge changes - filterMesh() operation for c2c guard

a3def93

WIP - runShapeQuery equivalent for proe

d8caa1d

IndexType over int

48882cd

Check file format and file type before reading

100fce7

Check for duplicate vertices in valid polyhedron check; add special t…

93fe2fe

…est case for clipping

WIP - move format check inside IntersectionShaper class instead of at…

66813c3

… the driver level

Add mixed proe/c2c unit test case

129b16f

Add proe-only test case with baseline

0eb779d

Properly guard the new proe tests in non-RAJA, non-Umpire case

fd260c0

Refactoring of prepareShapeQuery

67c9f8c

Use ArrayView over pointer copies

eda9dcc

Refactor runShapeQuery; make NUM_VERTS standard between Octs and Tets

4ce4d77

bmhan12 added 13 commits June 12, 2023 15:09

Cleanup

917ed8f

Put duplicate vertices check into kernel body to remove warning; bugf…

f42abd0

…ix wrong count

Unused

f263fd1

Make hasDuplicateVertices() function standalone with tolerance, remov…

50d2d43

…e duplicate logic in driver

Convert bounding boxes and hexes to Array

811a52b

Additional Array refactoring, but hanging on rzvernal...

c843ec6

Leave troublesome allocations as ptrs instead of Arrays

72bfd43

Refactor oct/tet members to use Array, refactor Discretize functions …

7cb0d8a

…to use Array to accomodate changes

Workaround changes for gcc@8.1.0 compiler

d340e93

Update notes

b2a7b1b

Wordsmithing

d799f4d

smithing, consistent style

f4c1907

Workaround for marching cubes for gcc@8.1.0~fortran compiler

dc5bd9e

bmhan12 force-pushed the feature/han12/shaping_proe branch from 375b17f to dc5bd9e Compare June 13, 2023 16:03

bmhan12 mentioned this pull request Jun 13, 2023

Remaining Issues with HIP #787

Open

8 tasks

bmhan12 merged commit d467ebe into develop Jun 13, 2023

bmhan12 deleted the feature/han12/shaping_proe branch June 13, 2023 17:00

bmhan12 mentioned this pull request Jul 19, 2023

Fix mesh tester example and add tests #1142

Merged

		@@ -882,11 +1005,12 @@ class IntersectionShaper : public Shaper
		axom::deallocate(m_hexes);

Conversation

bmhan12 commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kennyweiss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kennyweiss Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

bmhan12 Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kennyweiss Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

bmhan12 Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kennyweiss Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

kennyweiss Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

bmhan12 Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

agcapps left a comment

Choose a reason for hiding this comment

Uh oh!

rhornung67 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bmhan12 commented Jun 13, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bmhan12 commented Jun 6, 2023 •

edited

Loading

bmhan12 Jun 6, 2023 •

edited

Loading