Add triangulated surface distance benchmark #1052
Conversation
aprokop
changed the title
aprokop
force-pushed
the
benchmark_distance_ball
branch
from
0ecf36b to
e81f15c
Compare
aprokop
changed the title
aprokop
force-pushed
the
benchmark_distance_ball
branch
from
e81f15c to
9d69509
Compare
| Value const &value, | ||
| OutputFunctor const &out) const | ||
| { | ||
| using ArborX::Details::distance; |
There was a problem hiding this comment.
OK since we are not in examples/ but wondering whether we need to do something about it.
There was a problem hiding this comment.
Like what? We can simply remove it, I guess, as we control the geometries in the benchmark. But if a user looks here at any point, we may want to leave this around.
There was a problem hiding this comment.
Add a comment, discuss whether we want to officially support geometric functions, etc.
There was a problem hiding this comment.
Add a comment, discuss whether we want to officially support geometric functions, etc.
I don't understand what you are saying.
There was a problem hiding this comment.
Users are not allowed to reach into functionality that lives in the namespace Details:: and we should avoid showcasing doing so.
There was a problem hiding this comment.
You are right. We wouldn't want to show it here. But it doesn't detect distance from the arguments.
benchmarks/triangulated_surface_distance/triangulated_surface_distance.cpp
Outdated
Show resolved
Hide resolved
benchmarks/triangulated_surface_distance/triangulated_surface_distance.cpp
Outdated
Show resolved
Hide resolved
| auto a = Kokkos::numbers::phi_v<float>; | ||
| auto b = 1.f; |
There was a problem hiding this comment.
What about
| auto a = Kokkos::numbers::phi_v<float>; | |
| auto b = 1.f; | |
| auto const phi = Kokkos::numbers::phi_v<float>; | |
| auto const one = 1.f; |
There was a problem hiding this comment.
I would prefer not to. a and b are icosahedron parameter. We don't necessarily need to initialize them to phi and one, could do any scaling of those.
benchmarks/triangulated_surface_distance/triangulated_surface_distance.cpp
Show resolved
Hide resolved
| Kokkos::view_alloc(space, Kokkos::WithoutInitializing, | ||
| "Benchmark::points"), | ||
| n); | ||
| generatePointCloud(PointCloudType::filled_box, std::cbrt(n), random_points); |
There was a problem hiding this comment.
How do you control the volume in which points are being generated?
There was a problem hiding this comment.
I don't. I generate the points in std::cbrt(n) box. I thought about providing an argument for it, but I honestly don't see what it adds to the benchmark.
There was a problem hiding this comment.
I suppose not much as long as it stays centered on the origin
An alternative would be some configuration file. But I like cmd line arguments because it plays better with bash scripting. |
|
One thing I haven't considered is that distance to a surface is somewhat pathological case for a This is outside of the scope of the current PR, though. Simply an observation. |
| return std::make_pair(vertices, triangles); | ||
| } | ||
|
|
||
| /* Suvdivide every triangle into four |
There was a problem hiding this comment.
| /* Suvdivide every triangle into four | |
| /* Subdivide every triangle into four |
I stopped worrying about copyright, really. I think we should just stick the same copyright on all files without worrying about years, and only updated it with major releases. Too much hassle to track for individual files. |
| @@ -1,5 +1,5 @@ | |||
| /**************************************************************************** | |||
| * Copyright (c) 2017-2022 by the ArborX authors * | |||
| * Copyright (c) 2023 by the ArborX authors * | |||
This constructs a triangular tesselation of a sphere, then computes the distances from a randomly generated point cloud to the surface. User can specify the number of refinements of isocahedron and its radius, as well as the number of query points. It also optionally allows for saving the mesh in the legacy VTK format.