Add padding option to element reference coords #1221

sandro-elsweijer · 2024-08-16T11:12:30Z

Describe your changes here:
This PR introduces a padding to the reference coords. This way the element ref coords becomes consistend with the geometries and the element_from_ref_coord_function.
This is the result of the element ref coord discussion.

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 Makefiles
The code is well documented
All function declarations, structs/classes and their members have a proper doxygen documentation
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

Github action

The code compiles without warning in debugging and release mode, with and without MPI (this should be executed automatically in a github action)
All tests pass (in various configurations, this should be executed automatically in a github action)

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/tutorial and a Wiki article.

Licence

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

holke

Not a thorough review but skimmed over it. Looks great on the first glance.

Changed the comment describing padding thoughout, since "amount of padding" could be understood as "number of bytes" instead we mean "number of entries" (i.e. doubles).

src/t8_element.hxx

src/t8_forest/t8_forest_geometrical.h

src/t8_schemes/t8_default/t8_default_hex/t8_dhex_bits.h

src/t8_schemes/t8_default/t8_default_line/t8_default_line.hxx

src/t8_schemes/t8_default/t8_default_line/t8_dline_bits.h

src/t8_schemes/t8_default/t8_default_vertex/t8_default_vertex.hxx

src/t8_schemes/t8_default/t8_default_vertex/t8_dvertex_bits.h

test/t8_schemes/t8_gtest_element_ref_coords.cxx

Co-authored-by: Johannes Holke <[email protected]>

Davknapp

Looks good in general, just found an argument that should be const and some iterator-variables with meaningless names.

Davknapp · 2024-10-08T09:49:02Z

src/t8_forest/t8_forest_geometrical.h

-t8_forest_element_coordinate (t8_forest_t forest, t8_locidx_t ltree_id, const t8_element_t *element, int corner_number,
-                              double *coordinates);
+t8_forest_element_coordinate_from_corner_number (t8_forest_t forest, t8_locidx_t ltree_id, const t8_element_t *element,
+                                                 int corner_number, double *coordinates);


Suggested change

int corner_number, double *coordinates);

const int corner_number, double * const coordinates);

Can you change the documentation of *coordinates to "\param [in, out]" to make clear that no memory is allocated?

Davknapp · 2024-10-08T09:49:32Z

src/t8_forest/t8_forest_geometrical.h

@@ -52,8 +52,8 @@ t8_forest_get_dimension (const t8_forest_t forest);
 *                             the x, y and z coordinates of the vertex.
 */
 void
-t8_forest_element_coordinate (t8_forest_t forest, t8_locidx_t ltree_id, const t8_element_t *element, int corner_number,
-                              double *coordinates);
+t8_forest_element_coordinate_from_corner_number (t8_forest_t forest, t8_locidx_t ltree_id, const t8_element_t *element,


Suggested change

t8_forest_element_coordinate_from_corner_number (t8_forest_t forest, t8_locidx_t ltree_id, const t8_element_t *element,

t8_forest_element_coordinate_from_corner_number (const t8_forest_t forest, const t8_locidx_t ltree_id, const t8_element_t *element,

Davknapp · 2024-10-08T10:34:55Z

src/t8_forest/t8_forest_geometrical.h

 void
-t8_forest_element_from_ref_coords (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,
-                                   const double *ref_coords, const size_t num_coords, double *coords_out);
+t8_forest_element_coordinate_from_ref_coords (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,


Suggested change

t8_forest_element_coordinate_from_ref_coords (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,

t8_forest_element_coordinate_from_ref_coords (const t8_forest_t forest, const t8_locidx_t ltreeid, const t8_element_t *element,

Davknapp · 2024-10-08T10:35:07Z

src/t8_forest/t8_forest_geometrical.h

-                                   const double *ref_coords, const size_t num_coords, double *coords_out);
+t8_forest_element_coordinate_from_ref_coords (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,
+                                              const double *ref_coords, const size_t num_coords, const size_t padding,
+                                              double *coords_out);


Suggested change

double *coords_out);

double * const coords_out);

Davknapp · 2024-10-08T10:36:07Z

src/t8_forest/t8_forest_geometrical.h

-t8_forest_element_from_ref_coords_ext (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,
-                                       const double *ref_coords, const size_t num_coords, double *coords_out,
-                                       const double *stretch_factors);
+t8_forest_element_coordinate_from_ref_coords_ext (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,


Suggested change

t8_forest_element_coordinate_from_ref_coords_ext (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,

t8_forest_element_coordinate_from_ref_coords_ext (const t8_forest_t forest, const t8_locidx_t ltreeid, const t8_element_t *element,

Davknapp · 2024-10-08T10:53:57Z

src/t8_schemes/t8_default/t8_default_vertex/t8_default_vertex.hxx

   * \param [out] out_coords  The coordinates of the points in the
   *                          reference space of the tree.
   */
  virtual void
  t8_element_reference_coords (const t8_element_t *elem, const double *ref_coords, const size_t num_coords,
-                               double *out_coords) const;
+                               const size_t padding, double *out_coords) const;


Suggested change

const size_t padding, double *out_coords) const;

const size_t padding, double * const out_coords) const;

Davknapp · 2024-10-08T10:54:08Z

src/t8_schemes/t8_default/t8_default_vertex/t8_dvertex_bits.c

@@ -176,12 +176,13 @@ t8_dvertex_vertex_ref_coords (const t8_dvertex_t *elem, const int vertex, double

 void
 t8_dvertex_compute_reference_coords (const t8_dvertex_t *elem, const double *ref_coords, const size_t num_coords,
-                                     double *out_coords)
+                                     const size_t padding, double *out_coords)


Suggested change

const size_t padding, double *out_coords)

const size_t padding, double * const out_coords)

Davknapp · 2024-10-08T10:54:18Z

src/t8_schemes/t8_default/t8_default_vertex/t8_dvertex_bits.h

 * \param [out] out_coords An array of \a num_coords x 1 x double that
 * 		                     will be filled with the reference coordinates
 *                         of the points on the vertex (will be set to 0).
 */
 void
 t8_dvertex_compute_reference_coords (const t8_dvertex_t *elem, const double *ref_coords, const size_t num_coords,
-                                     double *out_coords);
+                                     const size_t padding, double *out_coords);


Suggested change

const size_t padding, double *out_coords);

const size_t padding, double * const out_coords);

Davknapp · 2024-10-08T10:55:42Z

example/IO/cmesh/gmsh/t8_load_and_refine_square_w_hole.cxx

@@ -94,7 +94,7 @@ t8_midpoint (t8_forest_t forest, t8_locidx_t which_tree, t8_eclass_scheme_c *ts,
    for (i = 0; i < 3; i++) {
      corner[i] = T8_ALLOC (double, 3);
      /* Get the coordinates of the elements  i-th vertex */
-      t8_forest_element_coordinate (forest, which_tree, element, i, corner[i]);
+      t8_forest_element_coordinate_from_corner_number (forest, which_tree, element, i, corner[i]);


Suggested change

t8_forest_element_coordinate_from_corner_number (forest, which_tree, element, i, corner[i]);

t8_forest_element_coordinate_from_corner_number (forest, which_tree, element, icorner, corner[icorner]);

Davknapp · 2024-10-08T10:56:11Z

src/t8_element.hxx

   * \param [out] out_coords  The coordinates of the points in the
   *                          reference space of the tree.
   */
  virtual void
  t8_element_reference_coords (const t8_element_t *elem, const double *ref_coords, const size_t num_coords,
-                               double *out_coords) const
+                               const size_t padding, double *out_coords) const


Suggested change

const size_t padding, double *out_coords) const

const size_t padding, double * const out_coords) const

sandro-elsweijer added 12 commits August 8, 2024 17:08

rename t8_forest_element_from_ref_coords

eab99bc

rename t8_forest_element_coordinate

558e4d7

adapt element ref coords interface and add padding option

f54bd58

add ref coord padding for hex

0f5cac9

add ref coord padding for line

5e04f66

add ref coord padding for triangle

32816ae

add ref coord padding for tet

4dc657c

add ref coord padding for prism

d02ec7c

add ref coord padding for pyra

490b73a

add ref coord padding for quad

cbf308f

add ref coord padding for vertex

35eb338

adapt ref coords test for padding

d7ccb21

sandro-elsweijer added enhancement Enhances already existing code priority: medium Should be solved within half a year workload: low Would take half a day or less labels Aug 16, 2024

sandro-elsweijer added 3 commits August 16, 2024 13:30

Merge remote-tracking branch 'origin/main' into enhancement-ref_coords

68ae09b

indent

d975a3a

indent

acaa546

holke requested changes Aug 16, 2024

View reviewed changes

Apply suggestions from code review

e052e29

Co-authored-by: Johannes Holke <[email protected]>

Davknapp self-assigned this Aug 19, 2024

Davknapp assigned sandro-elsweijer and unassigned Davknapp Sep 19, 2024

Merge branch 'main' into enhancement-ref_coords

84a3edd

sandro-elsweijer enabled auto-merge September 19, 2024 07:31

sandro-elsweijer assigned Davknapp and unassigned sandro-elsweijer Sep 19, 2024

Davknapp requested changes Oct 8, 2024

View reviewed changes

Davknapp assigned sandro-elsweijer and unassigned Davknapp Oct 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add padding option to element reference coords #1221

Add padding option to element reference coords #1221

sandro-elsweijer commented Aug 16, 2024 •

edited

Loading

holke left a comment

Davknapp left a comment

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

Davknapp Oct 8, 2024

	int corner_number, double *coordinates);
	const int corner_number, double * const coordinates);

	t8_forest_element_coordinate_from_corner_number (t8_forest_t forest, t8_locidx_t ltree_id, const t8_element_t *element,
	t8_forest_element_coordinate_from_corner_number (const t8_forest_t forest, const t8_locidx_t ltree_id, const t8_element_t *element,

	t8_forest_element_coordinate_from_ref_coords (t8_forest_t forest, t8_locidx_t ltreeid, const t8_element_t *element,
	t8_forest_element_coordinate_from_ref_coords (const t8_forest_t forest, const t8_locidx_t ltreeid, const t8_element_t *element,

	const size_t padding, double *out_coords) const;
	const size_t padding, double * const out_coords) const;

Add padding option to element reference coords #1221

Are you sure you want to change the base?

Add padding option to element reference coords #1221

Conversation

sandro-elsweijer commented Aug 16, 2024 • edited Loading

General

Tests

Github action

Scripts and Wiki

Licence

holke left a comment

Choose a reason for hiding this comment

Davknapp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandro-elsweijer commented Aug 16, 2024 •

edited

Loading