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

Restructure runscripts and related resources #181

Merged
merged 19 commits into from
Apr 12, 2021
Merged

Restructure runscripts and related resources #181

merged 19 commits into from
Apr 12, 2021

Conversation

MakisH
Copy link
Member

@MakisH MakisH commented Apr 10, 2021

Similarly to #169 and #166, this collects all the run scripts into common files, to reduce duplication.

  • OpenFOAM: separated between a "preparation" and an "execution" phase. The execution is same for every case, the preparation can differ. I did not find any reason to use a function, so I just call the general script from inside each case-specific script.
  • deal.II: no changes.
  • CalculiX: I only removed unnecesary/inconsistent comments and variables. Some cases with large meshes already set (default) values for OpenMP, I assume that these actually work and can be useful.
  • SU2: We only have one case, I left it hard-coded and simplified it.
  • FEniCS: In some cases, I added a run.sh or a clean.sh that was missing. I guess that some of the cases are not yet in the fully restructured form, but these are elements we need anyway. I have not checked if the runscripts run. I also softened a bit an existing .gitignore file for a FEniCS case.
  • Nutils: Similarly to FEniCS. I also extended the clean_nutils to remove all ./*.vtk files. Not tested.
  • code_aster: Only formatting and consistency changes.

All scripts have been validated with shellcheck.

Before merging:

  • Check documentation for renamed scripts: plot displacements, download meshes, run.sh / runFluid / Allrun, ...
  • Check documentation for 0.orig (moved to 0 in most cases) . What happens when we use the cleaning scripts of the foundation version?

After merging:

  • Update Quickstart (@davidscn would be a good review/cross-check that you apply the same changes there and I review the separate PR)

@MakisH MakisH self-assigned this Apr 10, 2021
@davidscn davidscn self-requested a review April 11, 2021 07:17
Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

Very nice! I guess your checklist keeps track of already checked run scripts and we can check tomorrow if everything works.

@MakisH MakisH marked this pull request as ready for review April 11, 2021 11:21
@MakisH MakisH requested a review from davidscn April 11, 2021 11:22
@MakisH
Copy link
Member Author

MakisH commented Apr 11, 2021

@davidscn ready for review! If you can easily run something with Nutils/FEniCS, please do. Other than than, any comments are welcome. Especially: did I forget anything? Is there anything in the documentation to fix?

@@ -40,7 +41,7 @@ clean_calculix() {
set -e -u
cd "$1"
echo "--- Cleaning up CalculiX case in $(pwd)"
rm -fv ./*.cvg ./*.dat ./*.frd ./*.sta ./*.12d spooles.out
rm -fv ./*.cvg ./*.dat ./*.frd ./*.sta ./*.12d spooles.out dummy
Copy link
Member Author

Choose a reason for hiding this comment

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

This dummy is generated at least in the heat exchanger CalculiX case, I have no idea where it comes from.

@@ -81,6 +82,7 @@ clean_nutils() {
set -e -u
cd "$1"
echo "--- Cleaning up Nutils case in $(pwd)"
rm -fv ./*.vtk
Copy link
Member Author

Choose a reason for hiding this comment

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

Now we delete all the VTK files inside Nutils cases. I guess not much can go wrong.

Copy link
Member

@davidscn davidscn left a comment

Choose a reason for hiding this comment

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

A two more comments:

  • the OpenFOAM run ends for me with the following message ../../tools/run-openfoam.sh: 17: .: openfoam-remove-empty-dirs.sh: not found
  • I created the deal.II run script as symbolic link similar to our cleaning scripts. Do you want to keep it in case of OpenFOAM as it currently is?

I will adjust the quickstart in #145 and we can check the remaining parts during the coding days :)

@MakisH
Copy link
Member Author

MakisH commented Apr 11, 2021

* the OpenFOAM run ends for me with the following message `../../tools/run-openfoam.sh: 17: .: openfoam-remove-empty-dirs.sh: not found `

Good catch, I fixed it by moving the cleaning part into run.sh as a separate step.

* I created the deal.II run script as symbolic link similar to our cleaning scripts. Do you want to keep it in case of OpenFOAM as it currently is?

In the way I see it, we cannot do this for OpenFOAM: some cases have different preparation steps and for all of them I create a named .foam file (opening two .foam files in ParaView should keep it clear what is what).

I will adjust the quickstart in #145 and we can check the remaining parts during the coding days :)

Then I don't wait for the Quickstart :).

@MakisH MakisH changed the title Restructure runscripts Restructure runscripts and related resources Apr 11, 2021
@MakisH
Copy link
Member Author

MakisH commented Apr 11, 2021

This PR is ready from my side. I also looked at the diff and it is as intended. Are there further comments or things to check?

@davidscn davidscn merged commit 07f770e into develop Apr 12, 2021
@davidscn davidscn deleted the runscripts branch April 12, 2021 05:37
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