Add tests for non-experimental API and subcycling.#1186
Conversation
There was a problem hiding this comment.
I only had a detailed look at one of the tests. Not outsourcing common parts of tests into generic functions, I guess is OK here. But this should not mean that we copy unnecessary code parts, see below. Goal should be that each individual test is as easy to understand as possible. Once it fails, we want to understand what the test does 😁
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
tests/serial/time/explicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Outdated
Show resolved
Hide resolved
|
@uekerman Do you think there are other tests that we would need here to make sure we don't break the classical subcycling? The only thing I can imagine is data initialization with subcycling. But this should already be covered by other tests. We could also shift some tests from other places over here. I just looked into precice/src/precice/tests/SerialTests.cpp Lines 1492 to 1504 in 703fbe4 If the main purpose of this test is not subcycling, then we should leave it as it is (in this PR). |
uekerman
left a comment
There was a problem hiding this comment.
Much easier to understand now 👍
@uekerman Do you think there are other tests that we would need here to make sure we don't break the classical subcycling? The only thing I can imagine is data initialization with subcycling. But this should already be covered by other tests.
Maybe testing a CompositionalCouplingScheme case? Not much more comes to mind.
Flite and Calculix testcase
That's still one from Bernhard. Not sure whether this one is about subcycling mainly. I guess you have a better overview.
What's the difference between DoNothingWithSubcycling and ReadWriteScalarDataWIthSubcycling? Why does the one only exist for serial coupling? Do we need both? If yes, maybe we could find better names.
tests/serial/time/implicit/parallel-coupling/ReadWriteScalarDataWithSubcycling.cpp
Show resolved
Hide resolved
Removed this test in 381c7de. I was hesitant about removing an existing test, but if you also don't really see a purpose for this test anymore, we can remove it. I think the test |
Opened an issue. I think we should delete it, because I don't really see why we need it. #1214 |
Added a very simple for-sake-of-completeness test in 29c6ae2. Doing something more exciting would require rewriting most of the test and we are testing compositional coupling schemes somewhere else. I think to make sure that subcycling does not do something strange this should be enough (e.g. counting iterations twice or not at all, if a compositional scheme is used). |
Main changes of this PR
Rearrange existing tests for subcycling and add new ones for different coupling schemes.
Motivation and additional information
Adding these tests helps to ensure that #1029 does not introduce a regression or accidentally adds new features, if
experimental=false.Author's checklist
make changelogif there are user-observable changes since the last release. (no user-side changes)make formatto ensure everything is formatted correctly.Reviewers' checklist