Improvement: Remove strong types from t8_3D_vec and t8_3D_point by lenaploetzke · Pull Request #2139 · DLR-AMR/t8code

lenaploetzke · 2026-02-02T13:43:23Z

Describe your changes here:
Closes #2130
Very open for suggestions

All these boxes must be checked by the AUTHOR before requesting review:

The PR is small enough to be reviewed easily. If not, consider splitting up the changes in multiple PRs.
The title starts with one of the following prefixes: Documentation:, Bugfix:, Feature:, Improvement: or Other:.
If the PR is related to an issue, make sure to link it.
The author made sure that, as a reviewer, he/she would check all boxes below.

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

The reviewer executed the new code features at least once and checked the results manually.
The code follows the t8code coding guidelines.
New source/header files are properly added to the CMake files.
The code is well documented. In particular, all function declarations, structs/classes and their members have a proper doxygen documentation. Make sure to add a file documentation for each file!
All new algorithms and data structures are sufficiently optimal in terms of memory and runtime (If this should be merged, but there is still potential for optimization, create a new issue).

Tests

The code is covered in an existing or new test case using Google Test.
The code coverage of the project (reported in the CI) should not decrease. If coverage is decreased, make sure that this is reasonable and acceptable.
Valgrind doesn't find any bugs in the new code. This script can be used to check for errors; see also this wiki article.

If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):

Should this use case be added to the github action?
If not, does the specific use case compile and all tests pass (check manually).

Scripts and Wiki

If a new directory with source files is added, it must be covered by the script/find_all_source_files.scp to check the indentation of these files.
If this PR introduces a new feature, it must be covered in an example or tutorial and a Wiki article.

License

The author added a BSD statement to doc/ (or already has one).

codecov · 2026-02-02T14:10:07Z

Codecov Report

❌ Patch coverage is 51.28205% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.89%. Comparing base (9f94c4e) to head (0e1c80b).

Files with missing lines	Patch %	Lines
src/t8_types/t8_vec.cxx	51.51%	16 Missing ⚠️
src/t8_types/t8_vec.hxx	40.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2139      +/-   ##
==========================================
- Coverage   77.95%   77.89%   -0.06%     
==========================================
  Files         113      112       -1     
  Lines       19089    19021      -68     
==========================================
- Hits        14881    14817      -64     
+ Misses       4208     4204       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…reasonable)

src/t8_types/t8_vec.hxx

sandro-elsweijer · 2026-02-05T15:27:01Z

src/t8_types/t8_vec.hxx

+/** Type alias for a 2D point.
 */
-using t8_2D_vec = t8_vec<2>;
+typedef std::array<double, 2> t8_2D_point;


What you you think, do we still need the differentiation between vectors and points or is a general dimensional type sufficient?

We could keep the vector name, since a vector is not only a vector in a geometric sense, but also a one-dimensional tensor.

No i also think that one dimensional object is enough. And yes i agree that we can just name it t8_vec and not t8_dimensional. I just defined both because we have so many usages of t8_3D_point in t8code...

sandro-elsweijer · 2026-02-05T15:28:09Z

src/t8_types/t8_vec.hxx

+template <typename T>
+concept T8ContainerdoubleType = requires (T t) {


Suggested change

template <typename T>

concept T8ContainerdoubleType = requires (T t) {

template <typename TType>

concept T8ContainerDoubleType = requires (TType type) {

Renamed to T8ContainerType and deleted the other one

src/t8_types/t8_vec.hxx

test/t8_gtest_custom_assertion.hxx

…t8_3d_point

sandro-elsweijer · 2026-02-09T13:53:20Z

src/t8_types/t8_vec.hxx

-  */
-template <typename T, std::size_t Expected = static_cast<std::size_t> (-1)>
-concept T8VecType = requires { typename std::remove_cvref_t<T>::tag; } && requires {
+concept T8ContainerType = requires (TType t) {


Since we use cpp20 now, we can also use ranges for this :)
This is more open and checks everything automatically

template <typename TType> concept T8Range = std::ranges::input_range<TType> && std::convertible_to<std::ranges::range_value_t<TType>, double>;

sandro-elsweijer · 2026-02-09T13:56:01Z

src/t8_types/t8_vec.hxx

 static inline void
 t8_cross_3D (const TVecX &vec_x, const TVecY &vec_y, TVecCross &cross)
 {
+  T8_ASSERT ((vec_x.size () == 3) && (vec_y.size () == 3));


Not all containers have a function named size (). Ranges is the container agnostic way.
cross needs to be big enough for the output values.

Suggested change

T8_ASSERT ((vec_x.size () == 3) && (vec_y.size () == 3));

T8_ASSERT ((std::ranges::distance(vec_x) >= 3) && (std::ranges::distance(vec_y) >= 3) && (std::ranges::distance(cross) >= 3));

sandro-elsweijer · 2026-02-09T13:57:07Z

src/t8_types/t8_vec.hxx

+template <T8ContainerType TVecX, T8ContainerType TVecY, T8ContainerType TVecDiff>
 constexpr void
 t8_diff (const TVecX &vec_x, const TVecY &vec_y, TVecDiff &diff)
 {


Size check is missing

sandro-elsweijer · 2026-02-09T13:58:31Z

src/t8_types/t8_vec.hxx

 static inline void
 t8_normal_of_tri (const TVecP1 &p1, const TVecP2 &p2, const TVecP3 &p3, TVecNormal &normal)
 {
+  T8_ASSERT ((p1.size () == 3) && (p2.size () == 3) && (p3.size () == 3));


Suggested change

T8_ASSERT ((p1.size () == 3) && (p2.size () == 3) && (p3.size () == 3));

T8_ASSERT ((std::ranges::distance(p1) >= 3) && (std::ranges::distance(p2) >= 3) && (std::ranges::distance(p3) >= 3));

sandro-elsweijer · 2026-02-09T14:00:24Z

src/t8_types/t8_vec.hxx

 static inline void
 t8_orthogonal_tripod (const TVecV1 &v1, TVecV2 &v2, TVecV3 &v3)
 {
+  T8_ASSERT (v1.size () == 3);


v2 and v3 still need to be big enough to hold the output values.

Suggested change

T8_ASSERT (v1.size () == 3);

T8_ASSERT ((std::ranges::distance(v1) >= 3) && (std::ranges::distance(v2) >= 3) && (std::ranges::distance(v3) >= 3));

sandro-elsweijer · 2026-02-09T14:03:25Z

src/t8_types/t8_vec.hxx

 static inline double
 t8_cross_2D (const TVecX &vec_x, const TVecY &vec_y)
 {
+  T8_ASSERT ((vec_x.size () == 2) && (vec_y.size () == 2));


Not all containers have a function named size (). Ranges is the container agnostic way.
Also it would be nice to check if the containers have at least the right size. In t8code we also sometimes use 3D containers to store 2D values and if this assertion is too limited, we cannot use this function.

Suggested change

T8_ASSERT ((vec_x.size () == 2) && (vec_y.size () == 2));

T8_ASSERT ((std::ranges::distance(vec_x) >= 2) && (std::ranges::distance(vec_y) >= 2));

remove strong types

e4bba3f

lenaploetzke linked an issue Feb 2, 2026 that may be closed by this pull request

Remove strong types from t8_3D_vec and t8_3D_point #2130

Open

doxygen

6e271b3

delete 3D and 2D concepts because of clangformat (and this is indeed …

f5e6e7b

…reasonable)

sandro-elsweijer requested changes Feb 5, 2026

View reviewed changes

sandro-elsweijer assigned lenaploetzke Feb 5, 2026

lenaploetzke added 6 commits February 6, 2026 11:18

remove viewer

a8efc9a

rename all t8_points to t8_vec

15d4e22

remove second concept and rename

2e55399

review

690c11e

Merge branch 'main' into 2130-remove-strong-types-from-t8_3d_vec-and-…

e3459a2

…t8_3d_point

missed thing by merge

0e1c80b

lenaploetzke requested a review from sandro-elsweijer February 6, 2026 13:42

lenaploetzke assigned sandro-elsweijer and unassigned lenaploetzke Feb 6, 2026

sandro-elsweijer requested changes Feb 9, 2026

View reviewed changes

		template <typename T>
		concept T8ContainerdoubleType = requires (T t) {

	T8_ASSERT ((vec_x.size () == 3) && (vec_y.size () == 3));
	T8_ASSERT ((std::ranges::distance(vec_x) >= 3) && (std::ranges::distance(vec_y) >= 3) && (std::ranges::distance(cross) >= 3));

	T8_ASSERT ((p1.size () == 3) && (p2.size () == 3) && (p3.size () == 3));
	T8_ASSERT ((std::ranges::distance(p1) >= 3) && (std::ranges::distance(p2) >= 3) && (std::ranges::distance(p3) >= 3));

	T8_ASSERT (v1.size () == 3);
	T8_ASSERT ((std::ranges::distance(v1) >= 3) && (std::ranges::distance(v2) >= 3) && (std::ranges::distance(v3) >= 3));

	T8_ASSERT ((vec_x.size () == 2) && (vec_y.size () == 2));
	T8_ASSERT ((std::ranges::distance(vec_x) >= 2) && (std::ranges::distance(vec_y) >= 2));

Conversation

lenaploetzke commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

General

Tests

Scripts and Wiki

License

Uh oh!

codecov bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lenaploetzke commented Feb 2, 2026 •

edited

Loading

codecov bot commented Feb 2, 2026 •

edited

Loading