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

Fix another issue with wrong edge tag update in swap23. #285

Merged
merged 6 commits into from
Oct 24, 2024

Conversation

Algiane
Copy link
Member

@Algiane Algiane commented Oct 17, 2024

The erroneous tag can be seen by adding checks on mesh consistency (call to MMG5_chkmesh) at the beginning of the loadbalancing function of ParMmg (PMMG_loadBalancing) and by calling the ls-CenIn-2 test case of ParMmg:
We try to perform a swap23 on a tetra with 2 boundary faces. Along the face at interface with the neighbour which with we perform the swap, the edge that doesn't belong to a boundary face has a wrong tag or no tag in the xtetra (due to a previous collapse that has added the xtetra but not updated the edge tag). It is not an issue as, in Mmg, we do not "trust" edge tags for edges not belonging to a boundary face. In the neighbour, the edge has the suitable tag (see attached picture).

path184

In this picture, the red and green faces belong to boundaries. The blue edge has not tag or errouneous tag (which is not an issue has it doesn't belongs to a boundary face). The grey tetra is the neighbour with which the swap is performed.

Without the fix introduced by this PR, the edge swap creates a tag inconsistency in one of the new tetra (in the sense that now, the edge without tag or with a wrong tag, may belong to a boundary face but still has an erroneous tag (for example if the grey tetra of the picture has 3 boundary faces for the faces not shared with the other tetra).

The current PR adds the suitable tag update by checking the tag in the edge shell (instead of using the tag of the xtetra 0).

The erroneous tag can be seen by adding chek on mesh consistency (call to MMG5_chkmesh) at the beginning of the loadbalancing function of ParMmg (PMMG_loadBalancing) and by calling the ls-CenIn-2 test case of ParMmg:
  - we try to perform a swap23 on a tetra with 2 boundary faces. Along the face at interface with the neighbour which with we perform the swap, the edge that doesn't belong to a boundary face has a wrong tag in the xtetra (due to a previous collapse that has added the xtetra). In the neighbour, the edge has the suitable tag. Leading to inconsistency.
@Algiane Algiane self-assigned this Oct 17, 2024
@Algiane Algiane added kind: bug error or fault part: mmg3d mmg3d specific labels Oct 17, 2024
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 50.45%. Comparing base (4267b33) to head (86de807).
Report is 9 commits behind head on develop.

Files with missing lines Patch % Lines
src/mmg3d/chkmsh_3d.c 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #285   +/-   ##
========================================
  Coverage    50.44%   50.45%           
========================================
  Files          177      177           
  Lines        47238    47239    +1     
  Branches     10271    10272    +1     
========================================
+ Hits         23830    23834    +4     
+ Misses       15676    15673    -3     
  Partials      7732     7732           

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

@corentin-prigent
Copy link
Contributor

Thanks !

@corentin-prigent corentin-prigent merged commit 37af05b into develop Oct 24, 2024
49 checks passed
@corentin-prigent corentin-prigent deleted the feature/pmmg-edge-tag-consistency branch October 24, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug error or fault part: mmg3d mmg3d specific
Development

Successfully merging this pull request may close these issues.

2 participants