Update code to use tag_t, coordinate_t, is_<geometry>_v #1033
Conversation
aprokop
changed the title
| template <typename Geometry> | ||
| using coordinate_type_t = typename coordinate_type<Geometry>::type; | ||
|
|
||
| // clang-format off |
There was a problem hiding this comment.
Why did you disable format?
One of the backslash in not aligned,
| #define DEFINE_GEOMETRY(name, name_tag) \ | ||
| struct name_tag{}; \ | ||
| template <typename Geometry> \ | ||
| struct is_##name : std::is_same<tag_t<Geometry>, name_tag>{}; \ |
There was a problem hiding this comment.
What's the point of defining the trait if you don't use it. You could define directly the helper variable template.
There was a problem hiding this comment.
Technically, ArborX::GeometryTraits is a user visible namespace. Removing is_point and such is not backwards compatible.
There was a problem hiding this comment.
It is still somewhat experimental and it is not documented. Also that is not a type that is intended to be specialized for user-defined geometry types.
I am not saying that we absolutely have to drop the trait type but that I think that there is nothing that prevent us to do so.
| std::is_same<Tag, SphereTag>{} || | ||
| std::is_same<Tag, TriangleTag>{} || | ||
| std::is_same<Tag, KDOPTag>{}, | ||
| static_assert(is_valid_geometry<Geometry>, |
There was a problem hiding this comment.
is_valid_geometry includes rays. Did you mean to add?
Now I see the message. So it is a yes.
Why so?
There was a problem hiding this comment.
Because Ray is a valid geometry. However, check_valid_geometry_traits and the tag are a bit weird. Tags are useful within ArborX, but less so for user defined geometries. So, the type check should really be only called for the geometries we control.
I know this patch is noisy. Still, I'd like to get it in.
is_<geometry>_vhelpers to avoidis_<geometry>::value_typecoordinate_type_t<geometry>helpers to avoidcoordinate_type<geometry>::typeis_valid_geometryGeometryTraitsforRay