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

Add linear cell interpolation mapping for 2D #1297

Merged
merged 90 commits into from Jun 15, 2022

Conversation

boris-martin
Copy link
Contributor

@boris-martin boris-martin commented May 18, 2022

Main changes of this PR

  • Added a new data mapping, "Volume Cell Interpolation", similar to NP but designed for volumetric coupling. Only supported in 2D for now. Usage: volume-cell-interpolation in the XML config and set up connectivitiy.
  • Added unit tests to check the correct interpolation, and integrations tests for conservative & consistent maps, in serial and parallel.

Motivation and additional information

Milestone of #468.
Followup of #1258

Author's checklist

  • I added a changelog file with make changelog if there are user-observable changes since the last release.
  • I ran make format to ensure everything is formatted correctly.
  • I sticked to C++14 features.
  • I sticked to CMake version 3.10.
  • I squashed / am about to squash all commits that should be seen as one.

Reviewers' checklist

@fsimonis : there is still the fall-back question to answer.
For now, we fall-back in NN when the vertex isn't inside a good triangle. However fall-back on edge could be useful in some edge cases (no pun intended).
If we consider the triangle formed by (0,0), (1,0) (0,1) and a point on (-eps, 0.5) with eps a small number, we clearly want to interpolate linearly in the edge. However it is not in the triangle thus falls back to NN

  • Does the changelog entry make sense? Is it formatted correctly?
  • Do you understand the code changes?

@fsimonis
Copy link
Member

Last comment from my side is that the name should be LinearCellInterpolationMapping to stay consistent with the other mappings.

src/mapping/LinearCellInterpolationMapping.cpp Outdated Show resolved Hide resolved
src/mapping/LinearCellInterpolationMapping.hpp Outdated Show resolved Hide resolved
src/mapping/LinearCellInterpolationMapping.cpp Outdated Show resolved Hide resolved
@@ -211,6 +211,25 @@ ProjectionMatch Index::findNearestProjection(const Eigen::VectorXd &location, in
}
}

ProjectionMatch Index::findCellInterpolation(const Eigen::VectorXd &location, int n)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are now falling back to nearest-projection by design. So this name is very misleading.

It could also be findCellOrProjection, but the name has to change.

src/query/Index.cpp Outdated Show resolved Hide resolved
tests/parallel/mapping-volume/testParallelSquare1To2.cpp Outdated Show resolved Hide resolved
tests/parallel/mapping-volume/testParallelSquare1To2.cpp Outdated Show resolved Hide resolved
tests/parallel/mapping-volume/testParallelSquare2To1.cpp Outdated Show resolved Hide resolved
Copy link
Member

@uekerman uekerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very clean already 👍
We still need more unit tests, also for the conservative variant. Probably sth you have on the radar anyway; I just mention it for completeness.

tests/parallel/mapping-volume/ParallelSquare2To1.cpp Outdated Show resolved Hide resolved
src/mapping/LinearCellInterpolationMapping.cpp Outdated Show resolved Hide resolved
tests/serial/mapping-volume/helpers.cpp Outdated Show resolved Hide resolved
@boris-martin
Copy link
Contributor Author

@uekerman all done except the unresolved conversation above.

@fsimonis fsimonis merged commit f1fb587 into precice:develop Jun 15, 2022
@fsimonis fsimonis changed the title [V2] Added "Volume Cell Interpolation" mapping support in 2D Add linear cell interpolation mapping for 2D Jun 15, 2022
@boris-martin boris-martin deleted the cleaner_volumetric_refactor branch July 1, 2022 21:23
@uekerman uekerman modified the milestones: Version 2.x.x, Version 2.5.0 Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A new feature, a new functionality of preCICE (from user perspective)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants