Allow initialize=true all data in serial coupling scheme by BenjaminRodenberg · Pull Request #1368 · precice/precice

BenjaminRodenberg · 2022-07-09T12:55:41Z

Main changes of this PR

Closes #1367

Motivation and additional information

See #1367

Todo

merge Waveform interpolation, read direction for serial implicit coupling #1352

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.16.3.
I squashed / am about to squash all commits that should be seen as one.

Reviewers' checklist

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

…ce/precice into 1149-restructure-integration-tests

…rm order zero.

…coupling

uekerman · 2022-07-12T11:39:41Z

src/cplscheme/config/CouplingSchemeConfiguration.cpp

+int CouplingSchemeConfiguration::getWaveformUsedOrder(std::string dataName) const
+{
+  int maxUsedOrder = -1;
+  for (const precice::impl::PtrParticipant &participant : _participantConfig->getParticipants()) {
+    for (auto &dataContext : participant->readDataContexts()) {
+      if (dataContext.getDataName() == dataName) {
+        int usedOrder = dataContext.getInterpolationOrder();
+        PRECICE_ASSERT(usedOrder >= 0); // ensure that usedOrder was set
+        return usedOrder;
+      }
+    }
+  }
+  return maxUsedOrder;
+}


I am a bit confused by the variable maxUsedOrder here.

Wouldn't it be "more correct" if we we pass the participant name as an argument here and return the order of that participant? In the end, we are only interested in the receiving participant, no?

That's a leftover. You are 100% right. I will quickly fix it.

…waveform order.

BenjaminRodenberg

I added a few comments.

BenjaminRodenberg · 2022-07-12T13:06:25Z

src/cplscheme/config/CouplingSchemeConfiguration.cpp

-      if (requiresInitialization && (_config.type == VALUE_SERIAL_EXPLICIT || _config.type == VALUE_SERIAL_IMPLICIT)) {
-        if (not _experimental) {
-          PRECICE_CHECK(not scheme.doesFirstStep(),
-                        "In serial coupling only second participant can initialize data and send it. "
-                        "Please check the <exchange data=\"{}\" mesh=\"{}\" from=\"{}\" to=\"{}\" initialize=\"{}\" /> tag in the <coupling-scheme:... /> of your precice-config.xml. You can bypass this restriction by activating the experimental interface for time interpolation. Please refer to the documentation for more details.",
-                        dataName, meshName, from, to, requiresInitialization);
-        }
-      }


We only define the waveform-order on the read-data. Therefore, we cannot check it here and raise a warning. Meaning: The first participant will not raise a warning.

Completely OK if only one of both participants raises a warning.

BenjaminRodenberg · 2022-07-12T13:09:08Z

src/cplscheme/config/CouplingSchemeConfiguration.cpp

-                        dataName, meshName, from, to, requiresInitialization);
-        }
+      // Checks for a serial coupling scheme, where initial data is received by first participant.
+      if (!scheme.doesFirstStep() && requiresInitialization && (_config.type == VALUE_SERIAL_EXPLICIT || _config.type == VALUE_SERIAL_IMPLICIT) && getWaveformUsedOrder(dataName) == 0) // the order of the statements is crucial! getWaveformUsedOrder should be called last, because it may run into an error, if the data is initialized, but not defined as read data (for example Integration/Serial/AitkenAcceleration)


This part is a bit tricky: Usually the second participant to which we are writing the initial data defines this as read-data. This is strictly not required (@uekerman, we briefly discussed this in the chat), but I think in most of the cases everything should work.

There is the highly theoretical case that a user does not define data that is initialized as read data. If this every happens, the error in getWaveformUsedOrder will be raised (and this might also be something worth knowing...).

BenjaminRodenberg · 2022-07-12T13:10:06Z

src/cplscheme/config/CouplingSchemeConfiguration.cpp

+      }
+    }
+  }
+  PRECICE_ERROR("Name \"{}\" not found. This is probably a bug. Please report it under https://github.com/precice/precice/issues/new/choose.", dataName);


I'm raising an error here, to make sure that the order that we receive from this function is actually valid.

uekerman

Minor tweaks.
Feel free to directly merge when done.

uekerman · 2022-07-12T14:21:44Z

src/cplscheme/config/CouplingSchemeConfiguration.cpp

-      if (requiresInitialization && (_config.type == VALUE_SERIAL_EXPLICIT || _config.type == VALUE_SERIAL_IMPLICIT)) {
-        if (not _experimental) {
-          PRECICE_CHECK(not scheme.doesFirstStep(),
-                        "In serial coupling only second participant can initialize data and send it. "
-                        "Please check the <exchange data=\"{}\" mesh=\"{}\" from=\"{}\" to=\"{}\" initialize=\"{}\" /> tag in the <coupling-scheme:... /> of your precice-config.xml. You can bypass this restriction by activating the experimental interface for time interpolation. Please refer to the documentation for more details.",
-                        dataName, meshName, from, to, requiresInitialization);
-        }
-      }


Completely OK if only one of both participants raises a warning.

src/cplscheme/config/CouplingSchemeConfiguration.cpp

src/cplscheme/config/CouplingSchemeConfiguration.hpp

Co-authored-by: Benjamin Uekermann <[email protected]>

* Add corresponding getters in Participant and ParticipantConfiguration * Raise assertion, if dataName or participantName is not found

* Add test Integration/Serial/InitializeData/ImplicitBoth where both participants initialize. * Allow initialize=true for both participants in serial-implicit coupling scheme. * Raise warning, if first participant in serial coupling scheme initializes data with waveform order zero. * Remove check for write data, because participant does not know about waveform order. Co-authored-by: Benjamin Uekermann <[email protected]>

fsimonis and others added 30 commits March 10, 2022 18:19

Auto generate test suites

30924ce

Update files

a18822f

Add reminder to run updateSourceFiles

a044834

Restructure Lifecycle/ImplicitFinalize

9a61dfb

Move TestConfigurationPeano and TestConfigurationComsol.

db4368c

:Restructure Parallel/lifecycle/Full

4fca910

Merge branch '1149-restructure-integration-tests' of github.com:preci…

6c257ac

…ce/precice into 1149-restructure-integration-tests

Further clean up and adding missing documentation.

da59a4a

Restructure Parallel/lifecycle/ConstructOnly

b43ec7b

Restructure Parallel/lifecycle/ConstructAndExplicitFinalize

1c948f6

Move TestExplicitWithDataScaling.

be6145e

Merge branch '1149-restructure-integration-tests' of github.com:preci…

43acc04

…ce/precice into 1149-restructure-integration-tests

Restructure parallel/TestFinalize

fff4cc3

Move Serial/MeshRequirements.

1413fe6

Merge branch '1149-restructure-integration-tests' of github.com:preci…

12f4b02

…ce/precice into 1149-restructure-integration-tests

Fix format.

0e0efad

Restructure Parallel/GlobalRBF

82b362f

Restructure LocalRBFPartitioning

d81cc9b

Restructure Parallel/CouplingOnLine

aafa557

Restructure parallel/TestBoundingBoxInit

f534614

Restructure Parallel/TestBoundingBoxInitTwoWay

2d3ee49

Restructure Parallel/NearestProjectionRePartitioning

bd4a7bf

Restructure Parallel/MasterSockets and Parallel/UserDefinedMPIComm

7049dff

Restructure Parallel/UserDefinedMPICommunicatorPetRBF

58789da

Restructure Parallel/ExportTimeSeries

9c1a3a9

Move multi coupling into its own suite.

6f6ed0a

Merge branch '1149-restructure-integration-tests' of github.com:preci…

52c5ab2

…ce/precice into 1149-restructure-integration-tests

Move MultiCouplingThreeSolvers.

4bbe656

Move lifecycle tests.

7ce0fe7

Fix includes.

3acf2f0

BenjaminRodenberg added 4 commits July 12, 2022 10:17

Raise warning.

f4bff08

Raise warning, if serial coupling scheme initializes data with wavefo…

4250e67

…rm order zero.

Merge branch 'develop-v3.0.0' into 1367-allow-initialize-true-serial-…

cdb50f2

…coupling

Reduce diff.

becf60b

BenjaminRodenberg added usability This issue will make preCICE easier for non-expert users breaking change Breaks backwards compatibility and users need to act labels Jul 12, 2022

BenjaminRodenberg mentioned this pull request Jul 12, 2022

Generally allow initialize=true for all coupling schemes and all participants #1367

Closed

uekerman reviewed Jul 12, 2022

View reviewed changes

BenjaminRodenberg added 3 commits July 12, 2022 14:29

Raise error.

ee3e2e8

Remove check for write data, because participant does not know about …

42b0c30

…waveform order.

Fix order of statements.

730da86

BenjaminRodenberg requested a review from uekerman July 12, 2022 13:05

BenjaminRodenberg commented Jul 12, 2022

View reviewed changes

BenjaminRodenberg added 2 commits July 12, 2022 15:12

Make it more explicit that function should only be called for read data.

c8ddeb8

Fix compile time error.

62b5dea

uekerman approved these changes Jul 12, 2022

View reviewed changes

BenjaminRodenberg and others added 3 commits July 16, 2022 14:11

Update src/cplscheme/config/CouplingSchemeConfiguration.cpp

8b9a97d

Co-authored-by: Benjamin Uekermann <[email protected]>

Provide participantName to getWaveformUsedOrder

193a431

* Add corresponding getters in Participant and ParticipantConfiguration * Raise assertion, if dataName or participantName is not found

Documentation.

b47a2b1

BenjaminRodenberg mentioned this pull request Jul 16, 2022

Update documentation for initialize precice/precice.github.io#186

Merged

1 task

BenjaminRodenberg merged commit feb5ca6 into develop-v3.0.0 Jul 17, 2022

BenjaminRodenberg deleted the 1367-allow-initialize-true-serial-coupling branch February 23, 2023 10:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Allow initialize=true all data in serial coupling scheme#1368

Allow initialize=true all data in serial coupling scheme#1368
BenjaminRodenberg merged 149 commits intodevelop-v3.0.0from
1367-allow-initialize-true-serial-coupling

BenjaminRodenberg commented Jul 9, 2022 •

edited

Loading

Uh oh!

uekerman Jul 12, 2022

Uh oh!

BenjaminRodenberg Jul 12, 2022

Uh oh!

BenjaminRodenberg left a comment

Uh oh!

BenjaminRodenberg Jul 12, 2022

Uh oh!

uekerman Jul 12, 2022

Uh oh!

BenjaminRodenberg Jul 12, 2022

Uh oh!

BenjaminRodenberg Jul 12, 2022

Uh oh!

uekerman left a comment

Uh oh!

uekerman Jul 12, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

BenjaminRodenberg commented Jul 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Main changes of this PR

Motivation and additional information

Todo

Author's checklist

Reviewers' checklist

Uh oh!

uekerman Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg left a comment

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

uekerman Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

BenjaminRodenberg Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

uekerman left a comment

Choose a reason for hiding this comment

Uh oh!

uekerman Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BenjaminRodenberg commented Jul 9, 2022 •

edited

Loading