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

Build cpp solvers as part of run #330

Merged
merged 3 commits into from
Apr 20, 2023

Conversation

fsimonis
Copy link
Member

The elastic-tube-1d cpp solvers require to be build inside build directories.
In-source builds don't work:

cd elastic-tube-1d/solid-cpp
cmake .
cmake --build .
./run.sh
Cannot find ./build/SolidSolver

This PR changes the run script to automatically configure and (re)build the solver.

@MakisH is this compatible with the VM?

Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

We would/should indeed need to remove the pre-building in the provisioning of the vm, here: https://github.com/precice/vm/blob/a37b13a50ade7658e63bd17bbe3230df085bba7f/provisioning/install-precice.sh#L43-L46

I understand the problem, but I am not sure I understand the motivation for adding the building inside the run.sh. Is this to ensure that the user does it correctly?

Seamless automation is better than documentation, so I agree with this change, as long as it works fine with running the run.sh again and again (without calling clean.sh), and as long as the output remains short and clear in that case.

I have not tested this, but it looks straight-forward. Remember to update the VM and the documentation.

elastic-tube-1d/fluid-cpp/run.sh Outdated Show resolved Hide resolved
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
Co-authored-by: Gerasimos Chourdakis <chourdak@in.tum.de>
@fsimonis
Copy link
Member Author

fsimonis commented Apr 4, 2023

@MakisH This change also means that you don't have to explicitly build as part of the vm provisioning any more.

@MakisH
Copy link
Member

MakisH commented Apr 4, 2023

@MakisH This change also means that you don't have to explicitly build as part of the vm provisioning any more.

But we still should remove that explicit building step:

We would/should indeed need to remove the pre-building in the provisioning of the vm, here: https://github.com/precice/vm/blob/a37b13a50ade7658e63bd17bbe3230df085bba7f/provisioning/install-precice.sh#L43-L46

@fsimonis
Copy link
Member Author

fsimonis commented Apr 4, 2023

But we still should remove that explicit building step:

@MakisH sorry, I somehow missed that part of your comment 😅

I understand the problem, but I am not sure I understand the motivation for adding the building inside the run.sh. Is this to ensure that the user does it correctly?

The problem is that our run script assumes an existing built solver in the build directory. As an example, calling cmake . && make in the solid-cpp dir doesn't work. So we either need to document this, extend run.sh to check more guesses, add a build.sh script, or build it directly in the run.sh.

Building it in the run script seemed the easiest to use from a user-perspective. Just ./run.sh to run.

@fsimonis fsimonis merged commit 362606a into precice:develop Apr 20, 2023
@fsimonis fsimonis deleted the build-cpp-solvers-in-run branch April 26, 2023 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants