Bugfix: Fix mesh handle tests for 5 processes - #2440
lenaploetzke wants to merge 6 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2440 +/- ##
=======================================
Coverage 82.65% 82.65%
=======================================
Files 132 132
Lines 21502 21515 +13
=======================================
+ Hits 17772 17783 +11
- Misses 3730 3732 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
spenke91
left a comment
There was a problem hiding this comment.
Thanks for the fix! I just have two minor remarks and am pretty sure I will accept the next iteration :-)
| if (!no_repartition && !this->set_partition_called ()) { | ||
| this->m_partition_for_coarsening = false; | ||
| t8_global_errorf ( | ||
| "WARNING: The mesh handle is intended to interpolate data after adaptation. " | ||
| "Therefore, repartitioning is required to happen AFTER interpolation. The balance function is called with " | ||
| "no_repartition = false, so the flag is set to true and partitioning is performed automatically after " | ||
| "interpolation.\n"); |
There was a problem hiding this comment.
Is this maybe a candidate for throwing an error rather than a warning? I'd argue that if it othersise (a) crashes, it is better to have a clean abort here or (b) computes garbage, it's even more important to abort here. My experience is that warnings have a tendency to be overlooked by the users.
There was a problem hiding this comment.
Hm i dont know. Isnt it in this case just fine to set the flag silently? Because this should always be the intend of the user in this case.
| EXPECT_EQ ((*mesh)[face.sides[iside].element_id].get_level (), elem_first.get_level () + 1) | ||
| << "MORTAR Small side must be one level finer."; | ||
| } | ||
| // TODO: If the neighbors of ghost work again, the following checks can be enables also for ghost large sides. |
There was a problem hiding this comment.
| // TODO: If the neighbors of ghost work again, the following checks can be enables also for ghost large sides. | |
| // TODO: If the neighbors of ghost work again, the checks above can be enabled also for ghost large sides. |
Or which checks do you mean?
There was a problem hiding this comment.
Actually the checks below, we theoretically can just delete the break.
Closes #2441
Describe your changes here:
The dg competence test for the mesh handle failed with 5 processes because the leaf neighbors of ghosts does not work as intended (See #2438). This test now works without using the neighbor of ghost elements.
The interpolation test failed because the mesh was partitioned before the data interpolation (implicit in the set_balance function because of the no_repartition=false flag). This cannot happen now.
All these boxes must be checked by the AUTHOR before requesting review:
Documentation:,Bugfix:,Feature:,Improvement:orOther:.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
Tests
If the Pull request introduces code that is not covered by the github action (for example coupling with a new library):
Scripts and Wiki
scripts/internal/find_all_source_files.shto check the indentation of these files.License
doc/(or already has one).