Improvement: Element data for mesh handle is now defined via mesh competence#2242
Improvement: Element data for mesh handle is now defined via mesh competence#2242lenaploetzke wants to merge 9 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2242 +/- ##
==========================================
- Coverage 82.59% 82.57% -0.02%
==========================================
Files 115 116 +1
Lines 18548 18556 +8
==========================================
+ Hits 15320 15323 +3
- Misses 3228 3233 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Davknapp
left a comment
There was a problem hiding this comment.
A first round of comments.
| const auto num_local_elements = this->underlying ().get_num_local_elements (); | ||
| const auto num_ghosts = this->underlying ().get_num_ghosts (); | ||
| T8_ASSERT (element_data.size () == static_cast<size_t> (num_local_elements)); | ||
| m_element_data.reserve (num_local_elements + num_ghosts); |
There was a problem hiding this comment.
I think we don't need to reserve the memory for the vector. std::move should only swap pointers here, so we don't need to allocate new memory. However you should mention in the comment, that element_data is moved, not copied.
There was a problem hiding this comment.
I thought i needed the reserve to avoid a reallocation if we later exchange ghost data (therefore i reserve more space then needed for element_data). What do you think?
There was a problem hiding this comment.
The move command often works by swapping the internal pointers. Therefore it is counterintuitive to reserve memory before swapping the pointers.
Maybe the name of the function should indicate too, that you are using a move assignement here.
if you want to ensure that you have more space than you currently have you should move and then reserve.
There was a problem hiding this comment.
Yeah i think i got it now. I did not rename the function but wrote an additional note in the documentation
Describe your changes here:
Closes #2185. Update of #2184 because something got messed up in git.
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).