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

Modifies solverdummy to include data transfer #750

Merged
merged 8 commits into from Jul 20, 2020
Merged

Modifies solverdummy to include data transfer #750

merged 8 commits into from Jul 20, 2020

Conversation

KyleDavisSA
Copy link
Contributor

@KyleDavisSA KyleDavisSA commented May 6, 2020 •
edited by fsimonis

This PR adds data transfer capability to the c, c++ and fortran solverdummies. This uses 3 vertices for transfering vector data and uses the same commands to run. They are also able to run in combination with each other.

This takes over the solverdummy extension for the previous version of preCICE which will be closed #540 .

Resolves #538

@KyleDavisSA KyleDavisSA mentioned this pull request May 6, 2020
Merged
uekerman
uekerman requested changes Jul 8, 2020
View reviewed changes
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.

A valuable contribution to the solver dummies I believe 👍

Before merging, we need to clean up a few things, however.

  • Could we rename Forces and Velocities to something neutral? DataOne, DataTwo maybe. Then, it would be more consistent with the participants' names, which are SolverOne etc and not FluidSolver currently.
  • I am not sure if we really need to print the data. Produces a lot of spam.
  • You mixed up "read/write iteration checkpoint" with isReadAvailable, isWriteDataRequired. I marked some occasions below.

examples/solverdummies/c/solverdummy.c Outdated Show resolved Hide resolved
examples/solverdummies/c/solverdummy.c Outdated Show resolved Hide resolved
examples/solverdummies/c/solverdummy.c Outdated Show resolved Hide resolved
examples/solverdummies/c/solverdummy.c Outdated Show resolved Hide resolved
examples/solverdummies/cpp/solverdummy.cpp Outdated Show resolved Hide resolved
examples/solverdummies/cpp/solverdummy.cpp Outdated Show resolved Hide resolved
examples/solverdummies/cpp/solverdummy.cpp Outdated Show resolved Hide resolved
@uekerman uekerman added this to the Version 2.1.0 milestone Jul 8, 2020
@MakisH
Copy link
Member

MakisH commented Jul 9, 2020

This will change the results of the system tests on purpose @Eder-K.

uekerman
uekerman approved these changes Jul 16, 2020
View reviewed changes
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.

Had to change quite some things, but should be good now.
@KyleDavisSA Please have a look at my final changes.

@uekerman uekerman mentioned this pull request Jul 16, 2020
Closed
BenjaminRodenberg
BenjaminRodenberg approved these changes Jul 16, 2020
View reviewed changes
Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment •
edited

Choose a reason for hiding this comment

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

Looks good 👍

One comment: I'm not sure whether we want/need this, but we could also include initializeData().

@uekerman
Copy link
Member

One comment: I'm not sure whether we want/need this, but we could also include initializeData().

Let's keep it simple. That's the story for another day (another PR).

@MakisH
Copy link
Member

MakisH commented Jul 19, 2020

Mainly for documentation purposes: what is the motivation behind changing from an explicit to an implicit scheme and from a 2D to a 3D interface?

@MakisH MakisH mentioned this pull request Jul 19, 2020
Merged
@MakisH
Copy link
Member

MakisH commented Jul 19, 2020

Ported the changes also to the Fortran module: precice/fortran-module#5
Ready for review, currently waiting for this one to get merged first.

@BenjaminRodenberg BenjaminRodenberg mentioned this pull request Jul 20, 2020
Closed
@IshaanDesai
Copy link
Member

The modified solver dummies for C, C++ and Fortran are tested with the new Python solver dummy and each combination works. Python solver dummy is modified in the PR: precice/python-bindings#48

@IshaanDesai IshaanDesai mentioned this pull request Jul 20, 2020
Closed
BenjaminRodenberg
BenjaminRodenberg reviewed Jul 20, 2020
View reviewed changes
Copy link
Member

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

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

Just checked

  • cpp/cpp
  • c/c
  • fortran/fortran
  • cpp/fortran
  • c/fortran
  • cpp/c

all combinations worked.

@BenjaminRodenberg
Copy link
Member

Also did the cross-check with precice/fortran-module#5 and all solverdummies worked.

@BenjaminRodenberg BenjaminRodenberg merged commit 8a1a00e into develop Jul 20, 2020
@BenjaminRodenberg BenjaminRodenberg deleted the newSolverDummy branch July 20, 2020 12:07
@precice-bot
Copy link

This pull request has been mentioned on preCICE Forum on Discourse. There might be relevant details there:

https://precice.discourse.group/t/highlights-of-the-new-precice-release-v2-1/274/1

@IshaanDesai IshaanDesai mentioned this pull request Jul 21, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BenjaminRodenberg BenjaminRodenberg BenjaminRodenberg approved these changes

@uekerman uekerman uekerman approved these changes

Assignees

@BenjaminRodenberg BenjaminRodenberg

Labels
None yet
Projects
None yet
Milestone
Version 2.1.0
Development

Successfully merging this pull request may close these issues.

Solver dummies do not include read and write functions
6 participants
@KyleDavisSA @MakisH @uekerman @IshaanDesai @BenjaminRodenberg @precice-bot