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

Add 3D tests for DistributedClosestPoints #1267

Merged
merged 5 commits into from
Feb 6, 2024

Conversation

gunney1
Copy link
Contributor

@gunney1 gunney1 commented Feb 4, 2024

Add 3D tests for DistributedClosestPoints

  • This PR adds a new test
  • It does the following:
    • Refactors the test code quest_distributed_distance_query_example.cpp to accomodate 3D meshes.
    • Extends the object mesh from a ring in 2D to a sphere in 3D.
    • Add switches controling the 3D configuration in the test.
    • Add the test to cmake.

Images from the 3D tests follow. Some of the object points lie outside the query mesh, so their colors aren't faded from the opacity of the mesh. The problem had a distance threshold of 0.3.

dcp3d
dcp3dA

This PR resolves #910

@gunney1 gunney1 added the Quest Issues related to Axom's 'quest' component label Feb 4, 2024
@gunney1 gunney1 self-assigned this Feb 4, 2024
@gunney1 gunney1 added the Testing Issues related to testing Axom label Feb 5, 2024
Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

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

This is looking nice @gunney1 -- if it's not too difficult, could you please post a screenshot and perhaps some performance stats (e.g. cpu vs. gpu vs. openmp) when they're ready?

};

// Output some global mesh size stats
{
Copy link
Member

Choose a reason for hiding this comment

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

Is it preferable to define each of these in a separate scope or reuse/rename the local variables?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The scope makes it clear that they're temporary variables, but reusing variables would shorten the code by a few lines.

Copy link
Member

@rhornung67 rhornung67 left a comment

Choose a reason for hiding this comment

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

A couple of comments to think about.

@gunney1 gunney1 marked this pull request as ready for review February 5, 2024 20:48
Copy link
Member

@agcapps agcapps left a comment

Choose a reason for hiding this comment

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

Thank you for the tests! One typo and a general style suggestion.

@gunney1 gunney1 merged commit a29fc73 into develop Feb 6, 2024
12 checks passed
@gunney1 gunney1 deleted the feature/gunney/closest-point-3d-tests branch February 6, 2024 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Quest Issues related to Axom's 'quest' component Testing Issues related to testing Axom
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add 3D test for DistributedClosestPoint
4 participants