Skip to content

Build cpp solvers as part of run#330

Merged
fsimonis merged 3 commits intoprecice:developfrom
fsimonis:build-cpp-solvers-in-run
Apr 20, 2023
Merged

Build cpp solvers as part of run#330
fsimonis merged 3 commits intoprecice:developfrom
fsimonis:build-cpp-solvers-in-run

Conversation

@fsimonis
Copy link
Copy Markdown
Member

@fsimonis fsimonis commented Mar 24, 2023

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?

MakisH
MakisH approved these changes Apr 3, 2023
View reviewed changes
Copy link
Copy Markdown
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.

@fsimonis @MakisH
Update elastic-tube-1d/fluid-cpp/run.sh
b530daa 
Co-authored-by: Gerasimos Chourdakis <[email protected]>
MakisH
MakisH reviewed Apr 4, 2023
View reviewed changes
@fsimonis @MakisH
Fix solid-cpp run
c8963b4 
Co-authored-by: Gerasimos Chourdakis <[email protected]>
@fsimonis
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Reviewers

@MakisH MakisH MakisH approved these changes

Assignees

@fsimonis fsimonis

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fsimonis @MakisH