Enhancement ghost query functions #1248

sandro-elsweijer · 2024-09-16T15:08:39Z

Describe your changes here:
I just drafted the code and it compiles without warning. I never executed the code. Giving it to our Hydrotec advisor @holke.

Note: The typos and indentation are from the base branch of this branch and I did not correct them since the base branch is still WIP. But this branch should be (if the new code works) usable by Hydrotec.

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.

License

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

holke

Thanks!
Only minor changes needed.

However, i suggest to take some part of the code and make a genera t8_element_array function out of it: Finding the index of an element in an element array.
t8_element_array_find (const t8_element_array_t *, const t8_element_t *)

Since we need this in other code places as well and should not duplicate the code.

holke · 2024-09-18T10:19:44Z

src/t8_forest/t8_forest_ghost.cxx

+  *loc_ghost_id = t8_forest_bin_search_lower (ghost_elements, linear_id, element_level);
+  if (*loc_ghost_id >= 0) {
+    /* The element was found */
+    return (const t8_element_t *) t8_sc_array_index_locidx (&(ghost_elements->array), *loc_ghost_id);


Suggested change

return (const t8_element_t *) t8_sc_array_index_locidx (&(ghost_elements->array), *loc_ghost_id);

return t8_element_array_index_locidx (ghost_elements, *loc_ghost_id);

holke · 2024-09-18T10:20:27Z

src/t8_forest/t8_forest_ghost.h

+ *
+ * \param [in] forest The forest object.
+ * \param [in] lghost_tree The local index of the ghost tree.
+ * \param [in] linear_id The local index of the element.


Suggested change

* \param [in] linear_id The local index of the element.

* \param [in] linear_id The linear id of the element.

holke · 2024-09-18T10:23:56Z

src/t8_forest/t8_forest_ghost.h

+ * \param [in] lghost_tree The local index of the ghost tree.
+ * \param [in] linear_id The local index of the element.
+ * \param [in] element_level The level of the element.
+ * \param [out] loc_ghost_id The local id of the ghost. -1 if no ghost was found.


We have to be very careful with the documentation and usage here.
The value of loc_ghost_id is the local index of the Ghost within its tree. (The return value of t8_ghost_get_ghost_id_in_tree)
It is not the id as in "The index in the elements + ghosts array that we use for example in ghost_exchange_data".

The current documention is in mho too unclear about that.

holke · 2024-09-18T10:24:24Z

src/t8_forest/t8_forest_ghost.h

@@ -69,6 +69,30 @@ t8_forest_ghost_get_tree_element_offset (t8_forest_t forest, t8_locidx_t lghost_
 t8_locidx_t
 t8_forest_ghost_tree_num_elements (t8_forest_t forest, t8_locidx_t lghost_tree);

+/** Retrieves the ghost index of an element in a specific ghost tree.


See comment on documentation below.

holke · 2024-09-18T10:28:15Z

src/t8_forest/t8_forest_ghost.cxx

+t8_locidx_t
+t8_ghost_get_ghost_id_in_tree (t8_forest_t forest, t8_locidx_t lghost_tree, t8_element_t *ghost_element)
+{
+  T8_ASSERT (t8_forest_is_committed (forest));


Add an assertion that the forest is not transitioned/does not have subelements.

The current version does not work with subelements since all subelements of the same elements have the same linear id, thus the search may find a different element, even if it exists.

We can either extend this search or (if there is not time) keep it but abort if it is used with subelements.

holke · 2024-09-18T10:30:01Z

src/t8_forest/t8_forest_private.cxx

@@ -52,3 +55,46 @@ t8_forest_get_tree_element_array_mutable (const t8_forest_t forest, t8_locidx_t
 {
  return (t8_element_array_t*) t8_forest_get_tree_element_array (forest, ltreeid);
 }
+
+/* TODO: should return t8_locidx_t */


Suggested change

/* TODO: should return t8_locidx_t */

holke · 2024-09-18T10:30:34Z

src/t8_forest/t8_forest_private.h

+/** Search for a linear element id (at forest->maxlevel) in a sorted array of
+ * elements. If the element does not exist, return the largest index i
+ * such that the element at position i has a smaller id than the given one.
+ * If no such i exists, return -1.


Suggested change

* If no such i exists, return -1.

* If no such i exists, return -1.

* \note This search does not work for subelements.

sandro-elsweijer added 3 commits September 16, 2024 17:07

move binary soearch to forest_private

f68206d

consting

933ac88

add ghost retrieval functions

1e0ef37

sandro-elsweijer added enhancement Enhances already existing code draft Enhance the visibility that this is a draft. labels Sep 16, 2024

sandro-elsweijer assigned holke Sep 16, 2024

Typos

3fee901

holke requested changes Sep 18, 2024

View reviewed changes

holke assigned sandro-elsweijer and unassigned holke Sep 18, 2024

sandro-elsweijer assigned holke and unassigned sandro-elsweijer Sep 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhancement ghost query functions #1248

Enhancement ghost query functions #1248

sandro-elsweijer commented Sep 16, 2024 •

edited

Loading

holke left a comment

holke Sep 18, 2024

holke Sep 18, 2024

holke Sep 18, 2024

holke Sep 18, 2024

holke Sep 18, 2024

holke Sep 18, 2024

holke Sep 18, 2024

holke Sep 18, 2024

	return (const t8_element_t ) t8_sc_array_index_locidx (&(ghost_elements->array), loc_ghost_id);
	return t8_element_array_index_locidx (ghost_elements, *loc_ghost_id);

	* \param [in] linear_id The local index of the element.
	* \param [in] linear_id The linear id of the element.

	* If no such i exists, return -1.
	* If no such i exists, return -1.
	* \note This search does not work for subelements.

Enhancement ghost query functions #1248

Are you sure you want to change the base?

Enhancement ghost query functions #1248

Conversation

sandro-elsweijer commented Sep 16, 2024 • edited Loading

General

Tests

Github action

Scripts and Wiki

License

holke 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

sandro-elsweijer commented Sep 16, 2024 •

edited

Loading