Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connectivity complete for periodic connectivities #136

Open
tim-griesbach opened this issue Apr 2, 2022 · 4 comments
Open

Connectivity complete for periodic connectivities #136

tim-griesbach opened this issue Apr 2, 2022 · 4 comments

Comments

@tim-griesbach
Copy link
Collaborator

tim-griesbach commented Apr 2, 2022

Description

As pointed out in #112 the p4est face balance algorithm requires corner connectivity data in some cases. In my tests this requirement was restricted to periodic connectivities.
One approach to overcome this situation could be to add the corner connectivity automatically by a function call. According to the name and documentation of the function p4est_connectivity_complete we may want to use this function.
In the documentation of p4est_connectivity_complete it is stated

Periodicity that is not inherent in the list of vertices will be lost.

but we could preserve this periodicity. In particular with respect to the
situation mentioned in #112 this would be beneficial if it is combined with the completion of the corner connectivity data.

Proposal

We could add the corner connectivity data by using the already existing function p4est_connectivity_join_faces.
This function fills in particular the corner connectivity data. Therefore, we can detect face periodicity by iterating over the tree_to_face array, reverting this periodicity in the tree_to_face array and then call p4est_connectivity_join_faces according to the prior detected face periodicity.
The result is a connectivity with complete corner connectivity data and therefore avoids the problem described in #112 and preserves the periodicity.
I tested this approach with manually selected parameters of p4est_connectivity_join_faces but so far not as a general function.
The proposal is for the branch prev3-develop.

cc @tisaac

@cburstedde
Copy link
Owner

It is possible that the vertices are chosen in a really funky way, contradicting with the face connectivity. How do we detect/resolve this situation? Would we even want to?

@tim-griesbach
Copy link
Collaborator Author

p4est_connectivity_is_valid ignores the content of the vertices array and furthermore p4est_connectivity_complete neither depends on the content of the vertices array nor changes it. p4est_connectivity_join_faces does not even depend at all on the vertices that means in contrast to the functions above it does not depend on the number of vertices.

Given the above described situation I would propose to not handle the situation of contradicting vertices since this coincides with the current handling in p4est and in particular with the way that p4est_connectivity_join_faces works.

@cburstedde
Copy link
Owner

Coming back here, prompted also by #237, extending the complete function to detect periodicity by faces would be a useful addition, if it does not incur any regression in non-periodic connectivities. So if @tim-griesbach: the changes are benign, we can add this to our list of tasks.

@cburstedde
Copy link
Owner

cburstedde commented Sep 21, 2023

In line with #112, I think your proposal is good. We should take care to update the documentation to make it clear that vertices play no role whatsoever in our connectivity and balance algorithms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants