Improvement: Remove strong types from t8_3D_vec and t8_3D_point#2139
Improvement: Remove strong types from t8_3D_vec and t8_3D_point#2139lenaploetzke wants to merge 3 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2139 +/- ##
==========================================
+ Coverage 77.56% 77.92% +0.35%
==========================================
Files 113 113
Lines 19025 19055 +30
==========================================
+ Hits 14757 14848 +91
+ Misses 4268 4207 -61 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| */ | ||
| template <std::size_t TDim, typename TType = double> | ||
| using t8_vec_view = T8Type<std::span<TType, TDim>, t8_vec_tag<TDim>, EqualityComparable, Swapable, RandomAccessible>; | ||
| using t8_vec = std::array<TType, TDim>; |
There was a problem hiding this comment.
This is a bit inconsistent. We have a using for vectors, but not for points and sometimes we have using and sometimes typedef.
| template <std::size_t TDim, typename TType = double> | ||
| using t8_vec_view = std::span<TType, TDim>; |
There was a problem hiding this comment.
This was only for the C interface, since we needed a non owning strong type. But now, we can just use a normal std::span and do not need a wrapper.
| /** Type alias for a 2D point. | ||
| */ | ||
| using t8_2D_vec = t8_vec<2>; | ||
| typedef std::array<double, 2> t8_2D_point; |
There was a problem hiding this comment.
What you you think, do we still need the differentiation between vectors and points or is a general dimensional type sufficient?
There was a problem hiding this comment.
We could keep the vector name, since a vector is not only a vector in a geometric sense, but also a one-dimensional tensor.
| template <typename T> | ||
| concept T8ContainerdoubleType = requires (T t) { |
There was a problem hiding this comment.
| template <typename T> | |
| concept T8ContainerdoubleType = requires (T t) { | |
| template <typename TType> | |
| concept T8ContainerDoubleType = requires (TType type) { |
| /** Concept for container types with any value type. | ||
| */ | ||
| template <typename T> | ||
| concept T8ContainerType = requires (T t) { | ||
| { | ||
| std::begin (t) | ||
| } -> std::input_iterator; | ||
| { | ||
| std::end (t) | ||
| } -> std::input_iterator; | ||
| }; |
There was a problem hiding this comment.
I understand why this is implemented, but since we are in a vector class with floating variables, it is okay if every function expects a floating point type.
|
|
||
| /** | ||
| * \brief Test if two 3D Dimensionaltors are equal with respect to a given precision | ||
| * \brief Test if two 3D containers are equal with respect to a given precision |
There was a problem hiding this comment.
| * \brief Test if two 3D containers are equal with respect to a given precision | |
| * \brief Test if two vectors are equal with respect to a given precision |
this does work with all dimensions and I think vector is better, but that's debatable
2 participants
Describe your changes here:
Closes #2130
Very open for suggestions
All these boxes must be checked by the AUTHOR before requesting review:
Documentation:,Bugfix:,Feature:,Improvement:orOther:.All these boxes must be checked by the REVIEWERS before merging the pull request:
As a reviewer please read through all the code lines and make sure that the code is fully understood, bug free, well-documented and well-structured.
General
Tests
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
Scripts and Wiki
script/find_all_source_files.scpto check the indentation of these files.License
doc/(or already has one).