-
-
Notifications
You must be signed in to change notification settings - Fork 128
Add tutorial of free flow over porous media #678
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
IshaanDesai
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.
Thanks for porting this case from the adapter repository. After correcting the executable name in free-flow-dumux/appl/CMakeLists.txt, I was able to run the case. The results look as expected.
...r-porous-media/images/tutorials-free-flow-over-porous-media-precice-config-visualization.png
Show resolved
Hide resolved
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 on the structure. I still want to look closer and run the case.
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.
This is shaping out nicely!
I still have some concerns regarding:
- The installation automation. This also did not work for me: installdumux.log.
- Similarly, the scripts in the root directory that seem to suggest a dumux-dumux combination only.
- Two small and hopefully easy to fix deviations from the naming conventions.
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.
This assumes the tutorial to be only the dumux-dumux combination, contradicting the modularity of the tutorials structure.
Why not move these steps into the run.sh of each case? We can add a step to check if the code has been built already (but Make would already act as a no-op in that case).
See the elastic-tube-1d as an example, or the Quickstart.
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.
This assumes the tutorial to be only the dumux-dumux combination, contradicting the modularity of the tutorials structure.
I do not see this argument. If there is another solver participant for this tutorial one day, it can be added next to the existing participant folders. Having DuMuX-specific scripts in the tutorial folder does not prevent the addition or operation for other solvers.
Why not move these steps into the
run.shof each case? We can add a step to check if the code has been built already (but Make would already act as a no-op in that case).
Both the DuMuX participants need the same DUNE modules, and compiling them together avoids a lot of code duplication.
| # Compile and move macro-dumux and micro-dumux executables to the participant folder level | ||
| ./compile-dumux-cases.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.
Besides this line, the rest of the script is a general DuMux installation script. Why have it here and not in the adapter? That would be similar to how we assume that OpenFOAM or CalculiX are already installed.
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.
This is because DuMuX is itself a DUNE module, and all modules need to be in the same directory. This is how the DUNE building mechanism is designed. Circumventing this would be very hacky, so we chose to download and install all necessary modules in the tutorial folder.
|
|
||
| To solve the flows with the DuMux framework, the necessary DUNE modules need to be downloaded and set up. This is done by running `sh setup-dumux.sh` in the tutorial folder. | ||
|
|
||
| Note that if an existing installation of DUNE modules is detected in a default location, this may lead to problems in running the `setup-dumux.sh` script. The script suppresses the environment variable `DUNE_CONTROL_PATH`. |
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.
Another reason to offload the installation to the user.
Is there a particular reason to do something special here?
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.
Yes, I find this note valuable, for reasons mentioned above.
Motivation: precice/dumux-adapter#52.
Checklist:
changelog-entries/<PRnumber>.md.