Add partition of unity RBF data mapping #1483
Conversation
davidscn
added
the
enhancement
Add basic working version Register only non-empty partitions Add index tree for center mesh Apply indexing for partition queries Use stored output vertex -> partition associations computed during computeMapping Make variables no member Add exports for center partitioning Replace size_t by vertexID in type Fix vector fill Clean-up includes and warn about non-matching output vertices Implement and document a tagging strategy Fiy typo Simplify partition BB logging Clean-up computeMapping function and factor out partitioning Remove empty partition map Set dead axis according to space dim Clarify data dimension vs space dimension Move partitioning strategy to separate file Work on partitioning Add tentative heuristic radius determination Some more logging and filtering of partitions Facot out filter logic Bugfix for vertex transform Make projection configurable Add global logger in namespace Remove unnecessary sqrt Parametrize overlap for the partitioning Add input projection as config parameter Fix test Add 2D version for duplication tagging Support vectorial data Improve debugging Bugfix for 2D cases A few tweaks and make it work for corner cases Fix a few warnings and remove static assert Fix typo Add more tests Add 3D test Add simple parallel test and exports in parallel More tests for the partitioning Remove obsolete code More modularity and synchronization of local vs global BBs Add a very simple parallel tagging algorithm First draft implementation for conservative pou mappings Use input mesh to construct partition distribution Resolve ambiguity of local variable and class member Add TODO regarding the tagging Add scalar conservative PoU tests Use free functions for begin and end Fixup Fixup fixup Use eigen methods Updates regarding new comm updates in preCICE preCICE compatibility updates pt2 Add conservative 2D vector test Add 3D conservative test Add a simple 3D vector consistent test Fix last test Further test fixes Add 3D conservative vector test Move weight computation into 'computeMapping' Make polynomial configurable
davidscn
force-pushed
the
pou-mapping
branch
from
ed5cda6
to
8d03ddc
Compare
There was a problem hiding this comment.
Looks promising already 👍
So far, I only had a big picture view and I did not yet try it out.
I struggle a bit with the name of the file CreatePartitioning and the namespace partitioner. Partition vs Partitioner vs Partitioning, how are they connected?
It gets even more complicated: we already have a package partition with a class Partition. Maybe we could find a different name here, also to clearly distinguish between parallel partition (mesh on one rank) and partitions meant here.
Maybe the connection between PoUMapping and CreatePartition could be a "has-a" connection?
CreatePartitioning and Partition could probably go into impl?
Let's add default values to all configuration attributes where we already know that there is a good safe choice.
Should we merge this before or after #1418?
Would be better to merge the bounding box refactoring first, the changes here are non-intrusive and can easily be adapted to the new class layout. |
|
While investigating the performance of the OpenFOAM-adapter using the 3D bending-flap tutorial, we observed a rather huge amount of time spent in the mapping part from the more refined solid side to the fluid side. Looking at the total amount of time in the fluid summary log, we had the following timings for the PETSc RBF mappings: advance/map.pet.computeMapping.FromSolid-writeToFluid-Mesh-Nodes | 1 | 408 | 408 | 408 | 408 | 0.00163 |
advance/map.pet.mapData.FromSolid-writeToFluid-Mesh-Nodes | 141 | 43943 | 526 | 17 | 311 | 0.176 |i.e., we have almost 44 seconds spent in the mapData (advance) call of preCICE. Using the POU method implemented here, we get the following timings: advance/map.pou.computeMapping.FromSolid-writeToFluid-Mesh-Nodes | 1 | 37 | 37 | 37 | 37 | 0.000215 |
advance/map.pou.mapData.FromSolid-writeToFluid-Mesh-Nodes | 141 | 495 | 3 | 2 | 3 | 0.00287 |i.e., the time spent in As a disclaimer: I still have to add an early exit from the 'computeMapping' function here in order to bail out at the right time when a rank is empty (has no interface vertices). For the measurement, I added an easy bail-out strategy, but I want think about it again and add a clean (maybe in the end even the same) version . |
src/mapping/Partition.hpp
Outdated
| unsigned int getNumberOfInputVertices() const; | ||
|
|
||
| /// The center coordinate of this partition | ||
| std::array<double, 3> getCenterCoords() const; |
There was a problem hiding this comment.
Is there any reason not to use Eigen::Vertex3D or Eigen::VertexXD instead?.
I only used this in the Vertex to prevent dynamic allocations and simplify the interface to boost.geometry.
There was a problem hiding this comment.
I see your point, but would rather delay adjustments in this direction. Here, I need to preserve compatibility towards vertex.rawCoords. Maybe something to adjust later on.
src/mapping/Partition.hpp
Outdated
| void setNormalizedWeight(double normalizedWeight, VertexID vertexID); | ||
|
|
||
| /// Compute the weight for a given vertex | ||
| double computeWeight(const mesh::Vertex &v) const; |
There was a problem hiding this comment.
This only requires coordinates
| double computeWeight(const mesh::Vertex &v) const; | |
| double computeWeight(const Eigen::VertorXD &v) const; |
There was a problem hiding this comment.
I see your point, but would rather delay adjustments in this direction. Here, I need to preserve compatibility towards vertex.rawCoords. Maybe something to adjust later on.
There was a problem hiding this comment.
I had a look over everything again with a focus on SphericalVertexCluster.hpp. I did not step through all details of CreateClustering.hpp.
I think we are in a good state now. Still a few things to tweak, but all minor.
Nice documentation in SphericalVertexCluster 👍
Does it make sense to also implement unit tests for estimateClusterRadius (and the other helper functions) ?
After merging, please add a new section in the mapping docs explaining the attributes with a picture.
| #ifndef NDEBUG | ||
| exportClusterCentersAsVTU(centerMesh); | ||
| #endif |
There was a problem hiding this comment.
Something to mention in the user docs
|
@uekerman regarding all the defaults: I added now defaults for the pum specific parameters. I'm just afraid that these settings will be considered as a silver bullet and people don't think and or modify these parameters: Example: going higher with the number of vertices-per-cluster might give more accurate results, but it's also more expensive. The values set here are just reasonable default values. |
|
This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there: https://precice.discourse.group/t/can-precice-be-used-for-volume-coupling/27/7 |
Main changes of this PR
Adds the partition of unity RBF mapping as a new mapping variant.
Motivation and additional information
There are a few details yet to be clarified and a few TODOs in the code, but I'm sure these changes are ready for a first review. We need to fix a user interface in the configuration file.
Post TODO: Adjust the auto-selection of our mapping-config.
Closes #1273.
Author's checklist
pre-commithook to prevent dirty commits and usedpre-commit run --allto format old commits.make changelogif there are user-observable changes since the last release.use-qr-decompositionto something more appropriate #1417)Reviewers' checklist