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.

Sign up for GitHub

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

Jump to bottom

Add selective synchronization #1589

Draft
wants to merge 29 commits into
base: develop
Choose a base branch
from
Draft

Add selective synchronization #1589

wants to merge 29 commits into from

Conversation

fsimonis
Copy link
Member

@fsimonis fsimonis commented Mar 2, 2023

Main changes of this PR

This PR adds

  • the dynamic attribute to provide-mesh, which allows to flag meshes as dynamic
  • logic for propagating this information to other participants
  • Coupling scheme self-awareness (what's my name)
  • Coupling scheme synchronization (which meshes have changed locally/remotely)
  • logic to determine if a coupling scheme needs to synchronize based on the above

Motivation and additional information

This handles only the synchronization. The information of which meshes change is not yet used.
This allows to further implement reinitalisation.

Closes #1293

Author's checklist

  • I used the pre-commit hook to prevent dirty commits and used pre-commit run --all to format old commits.
  • I added a changelog file with make changelog if there are user-observable changes since the last release.
  • I added a test to cover the proposed changes in our test suite.
  • For breaking changes: I documented the changes in the appropriate porting guide.
  • I sticked to C++17 features.
  • I sticked to CMake version 3.16.3.
  • I squashed / am about to squash all commits that should be seen as one.

Reviewers' checklist

  • Do you understand the code changes?

@fsimonis fsimonis self-assigned this Mar 2, 2023
@fsimonis fsimonis added the enhancement A new feature, a new functionality of preCICE (from user perspective) label Mar 2, 2023
@fsimonis fsimonis added this to the Version 3.0.0 milestone Mar 2, 2023
@fsimonis fsimonis requested a review from uekerman March 2, 2023 15:36
@fsimonis
Copy link
Member Author

fsimonis commented Mar 3, 2023 •
edited
Loading

This currently doesn't work properly. Checking based on the participant name alone doesn't cover all cases. Especially if multiple coupling schemes are mixed.

I need to use the exchange tags to deduce which meshes are used in a coupling. Multi schemes need to know this for all participants.
Then I can use the global info of Participant->dynamic received/provided mesh from the participant configuration and pass it to the schemes.
Schemes can then deduce if they are required to synchronize, based on which meshes are involved in the coupling.

@fsimonis
Copy link
Member Author

fsimonis commented Mar 5, 2023 •
edited
Loading

This works in these steps:

  • <provide-mesh name="MeshA" dynamic="true"/> sets the provided mesh context to dynamic
  • the participant configuration determines for all participants:
    • which receive mesh contexts are dynamic (provided is dynamic)
    • which meshes are transitively dynamic via mappings
    • which receive meshes are transitively dynamic (provided is transitively dynamic)
  • in the CouplingSchemeConfiguration, exchange tags now have an extended purpose
    • if data is exchanged via a dynamic mesh (fully or transitively) in either participant, then it marks the couplingscheme to require synchronization
    • multi-coupling schemes now use synchronization if any (!) participant exchanges data via a dynamic mesh.

CouplingScheme::isSynchronizationRequired() can now be used in the SolverInterfaceImpl to deduce if the intra-synchronization as well as first and second synchronizations require extra handling.

@davidscn davidscn mentioned this pull request Mar 8, 2023
Closed
8 tasks
@uekerman uekerman marked this pull request as ready for review March 27, 2023 12:00
uekerman
uekerman approved these changes Mar 27, 2023
View reviewed changes
Copy link
Member

@uekerman uekerman left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Very clean code 👌

I did not follow each and every detail, but the overall functionality looks good. It would, however, be good to have some dev docs on this overall functionality: what first and second synch is and how the transitive spread of dynamicity works. Could be a page in the dev docs to which we could link from the source code.

I think it would be good to also extend the unit test of MeshContext:

  • Cases with more than 2 participants (and compositional or multi coupling schemes)
  • Case with multiple mappings from or to the same mesh

Do (more) unit tests in cplscheme make sense?

src/precice/config/ParticipantConfiguration.hpp Show resolved Hide resolved
src/precice/config/ParticipantConfiguration.cpp Outdated Show resolved Hide resolved
src/precice/config/ParticipantConfiguration.cpp Show resolved Hide resolved
src/precice/impl/Participant.hpp Show resolved Hide resolved
src/cplscheme/BaseCouplingScheme.cpp Show resolved Hide resolved
src/cplscheme/BiCouplingScheme.cpp Outdated Show resolved Hide resolved
@fsimonis
Copy link
Member Author

I ran into trouble with Multi-Coupling cases.

@fsimonis fsimonis marked this pull request as draft June 20, 2023 12:27
@fsimonis fsimonis modified the milestones: Version 3.0.0, Version 3.x.x Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uekerman uekerman uekerman approved these changes

Assignees

@fsimonis fsimonis

Labels
enhancement A new feature, a new functionality of preCICE (from user perspective)
Projects
None yet
Milestone
Version 3.x.x
Development

Successfully merging this pull request may close these issues.

Introduce tag for use of adaptive meshes
2 participants
@fsimonis @uekerman