Feature: extend the partition algorithm to handle weights

[dutkalex]

Describe your changes here:
This PR makes it possible to define element weights for the partitioning algorithm. This is achieved via a callback pointer. In the null case, the old unweighted algorithm is used. This introduces one breaking change in the public API, in the t8_forest_set_partition function signature. All the client code in the repo has been updated, but this will require an update of external user code: t8_forest_set_partition(forest, set_from, set_for_coarsening) => t8_forest_set_partition(forest, set_from, set_for_coarsening, nullptr)

Design tradeoffs:

• The new algorithm replaces the old one, even if no weight function is provided. Keeping the old algorithm around could possibly result in slightly better performance since it requires less work, but this would come at the cost of increased maintenance and the need for duplicated testing to cover both code paths. Given that the partition algorithm is not a performance bottleneck in practice, the single code path solution was preferred. The final design includes the unweighted case as a fast "early exit" case.
• In the general case, a single pass algorithm is not possible because the total weight of the forest wan't be known a priori and must be computed (first traversal of the forest) to then be able to define the new partition boundaries (second traversal). This means that each element weight is computed twice with the current implementation. If the weight function is expensive to evaluate, this could cause performance problems. There are basically two ways to work around this: the results could either be cached during the first evaluation (this means that we need to allocate a temporary array), or the weights could alternatively be computed and stored in an array ahead-of-time by the user, before calling the partition function. In the second case the user then simply provides a weight function that looks up the precomputed weights. This second option was preferred because it gives the user greater flexibility, and because it offers optimal performance in the case where the weight function is cheap to evaluate which is probably the default case ("Don't pay for what you don't use" principle).

All these boxes must be checked by the AUTHOR before requesting review:

• The PR is small enough to be reviewed easily. If not, consider splitting up the changes in multiple PRs.
• The title starts with one of the following prefixes: Documentation:, Bugfix:, Feature:, Improvement: or Other:.
• If the PR is related to an issue, make sure to link it.
• The author made sure that, as a reviewer, he/she would check all boxes below.

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 CMake files.
• The code is well documented. In particular, 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.
• The code coverage of the project (reported in the CI) should not decrease. If coverage is decreased, make sure that this is reasonable and acceptable.
• Valgrind doesn't find any bugs in the new code. This script can be used to check for errors; see also this wiki article.

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 or tutorial and a Wiki article.

License

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

[holke]

Thanks again for this addition!

Since 95% of our users do not use weighted partitioning but the classical one and the weighted does add overhead i would argue we should at least skip the weight computation loop in standard mode.
Or keep the standard version as is - but i also see your point of avoiding code duplication which is very important.

[dutkalex]

Since 95% of our users do not use weighted partitioning but the classical one and the weighted does add overhead i would argue we should at least skip the weight computation loop in standard mode.

That's an interesting idea. Maybe we can get the best of both worlds with this kind of "fast-forward" behavior in the simple case. I will look into that.

[dutkalex]

I wonder what should be the input of the weight function. Currently it takes a forest, a local tree id and an element id (within that tree). I'm not quite convinced this is really usable in practice however, especially if the forest has just been refined or coarsened prior to entering the partition algorithm. Do you have ideas on what would be the right function signature in the general case?
Since the weight function is always used on a committed forest, then all the non mutating public API functions can be used by this weight function to query for additional information if needed.

[codecov]

Codecov Report

❌ Patch coverage is 38.46154% with 48 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.88%. Comparing base (4a06e41) to head (25837e6).

Files with missing lines	Patch %	Lines
src/t8_forest/t8_forest_partition.cxx	39.18%	45 Missing ⚠️
src/t8_forest/t8_forest.cxx	25.00%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1889      +/-   ##
==========================================
- Coverage   77.05%   76.88%   -0.17%     
==========================================
  Files         112      112              
  Lines       18959    19016      +57     
==========================================
+ Hits        14608    14620      +12     
- Misses       4351     4396      +45     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[dutkalex]

I have refactored to use t8_shmem_array_allgatherv in the weighted case as you suggested @holke, but it seems like I'm still missing something to make this work...
https://github.com/dutkalex/t8code/blob/weighted-partition/src/t8_forest/t8_forest_partition.cxx#L519

[spenke91]

Thanks a lot for your contribution @dutkalex ! I am out of office this week, but looking forward to reviewing it next week 👍

[spenke91]

Thank you so much @dutkalex for your contribuition! I like the new feature a lot and as discussed, it seems to work flawlessly. As you can see, I only have minor remarks, which are mostly on how to pass the weight function to the forest.

Don't hesitate to contact me in Mattermost if you want to discuss something! :-)

[spenke91]

Thank you for the quick replies! I discussed with @holke today about this PR and here's what we think.

[spenke91]

Thank you for your replies and once again for your contribution 👍

[dutkalex]

Hi @spenke91!
I found some time to come back to this PR and make the changes you requested (i.e. rollback the breaking API change in favor of an additional function to set the weights callback, and rollback the test hack). I also rebased the PR on the head of main, and fixed the conflicts with the custom offsets feature that recently got merged. IIRC, you implemented this feature, so please let me know if you have questions/comments/complaints on the refactoring.
Btw, feel free to ignore this message. I hope you're having a good time during these winter holidays.
I wish you - and the t8code team - all the best for the new year!