-
-
Notifications
You must be signed in to change notification settings - Fork 132
Water Hammer Tutorial #660
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
base: develop
Are you sure you want to change the base?
Conversation
MakisH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the clean PR! See some first comments on the structure.
I have not yet looked into the codes or the parameters, I would do that once the locations of the files have converged, to not lose track of the suggestions and changes.
The 3D and 3D-3D cases run, anything with the 1D code does not at the moment, but this is probably related to my system (see also #648 (comment)).
Please add some content to the PR description as well.
water-hammer/results/I/p
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The results/ folder is very nice for your thesis, but not something for the tutorials. Eventually, this directory (with a copy of the rest) would be something for Zenodo or a separate repository on the LRZ GitLab.
We could also then take these application case guidelines into account: https://precice.org/community-guidelines-application-cases.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also add the DOI here (not yet available, let's look at it together): https://doi.org/10.5281/zenodo.18220952
water-hammer/images/plotting_all.py
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Visualization scripts (for tutorials) should be in the root directory of the tutorial. See the structure of a tutorial in https://precice.org/community-contribute-to-precice.html#structure-of-a-tutorial
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this visualization script now?
water-hammer/case-1d-3d/fluid3d-openfoam/constant/thermodynamicProperties
Outdated
Show resolved
Hide resolved
| <precice-configuration experimental="true"> | ||
| <log> | ||
| <sink | ||
| filter="%Severity% >= trace and %Rank% = 0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the final state, the logging level should be the default:
| filter="%Severity% >= trace and %Rank% = 0" | |
| filter="%Severity% > debug and %Rank% = 0" |
(same in all files)
water-hammer/README.txt
Outdated
| In one terminal, execute | ||
|
|
||
| ```bash | ||
| cd case-1d/fluid1d-python-uncoupled && ./run.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running with scipy works but throws a (maybe useful) warning:
NutilsDeprecationWarning: providing evaluation arguments as keyword arguments is deprecated, please use the "arguments" parameter instead
In /home/gc/repos/precice/tutorials/water-hammer/case-1d/fluid1d-python-uncoupled/Fluid1D.py:107
DeprecationWarning: Conversion of an array with ndim > 0 to a scalar is deprecated, and will error in future. Ensure you extract a single element from your array before performing this operation. (Deprecated NumPy 1.25.)
In /home/gc/repos/precice/tutorials/water-hammer/case-1d/fluid1d-python-uncoupled/Fluid1D.py:111
I modified run.sh to NUTILS_MATRIX=scipy python3 Fluid1D.py and I added scipy to requirements.txt to get this.
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
REmove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
Remove decorative headers
MakisH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some first comments. I still need to run the case and look into the Nutils scripts.
water-hammer/README.md
Outdated
| In that study, the cross-section of the pipe was squared. It has been changed to a circular cross-section in the present tutorial. | ||
| `1D` denotes the reduced-order domain (e.g., a Nutils solver) and `3D` denotes the full 3D CFD domain (e.g., OpenFOAM). | ||
|
|
||
| The problem consists of a straight pipe of length `L = 1000 m` and diameter `D = 2 m`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also use math mode, if you want: https://precice.org/docs-meta-cheatsheet.html#latex-math-syntax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to make these inline. Note also some \text{1D} and similar above.
| keywords: OpenFOAM, Nutils, preCICE, geometric-multiscale, fluid, transient | ||
| summary: The Partitioned Water Hammer tutorial simulates unsteady pressure wave propagation in pipe systems using different 1D and 3D configurations coupled via preCICE. | ||
| --- | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also needs to point to the files, see #704
|
Update: I resolved a few conversations that have already been addressed. What is still open is still relevant. This looks already very close to merging. Together, let's polish the |

Checklist:
changelog-entries/<PRnumber>.md.