-
Notifications
You must be signed in to change notification settings - Fork 1
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
Copy soil layer setting csv to input folder + add a BMI MODFLOW coupling notebook #104
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
@MostafaGomaa93 Well done 👍 , nice to see the complete workflow of coupling. Thanks for uploading the required files to Zenodo; it's great to see that the example data for NL-Loo
is now available. I successfully ran the notebook using the NL-Loo example data, but I cannot run the notebook for a different site. Because I couldn’t find any instructions on this in the introductory cell of the notebook or within the files uploaded to Zenodo. Can you please add the instructions here in the notebook or/and a preprocessing script to zenodo?
- In the intro section of your notebook:
- Replace the link to the documentation of pyStemmusScope package with the latest link as https://pystemmusscope.readthedocs.io/en/latest/.
- Provide the exact link to STEMMUS_SCOPE executable file and not the repository e.g. https://github.com/EcoExtreML/STEMMUS_SCOPE/blob/main/run_model_on_snellius/exe/STEMMUS_SCOPE
- The same for MODFLOW 6 executable files, provide the exact links.
- The paths to the files:
- The codes in cells 1, 1.1, and 1.2 are difficult to read and follow. Additionally, the notebook doesn't need to handle tasks like copying files for users. Instead, users should simply be required to specify the following items:
modflow_working_directory = "path_to_modflow_dir/mf6_model_base"
modflow_config_file = f"{modflow_working_directory}/mfsim.nam"
modflow_exe_file = "path_to_modelfolw_executables/libmf6.so" # for window .dll, otherwise .so
stemmus_scope_config_file = "path_to_stemmus_scope_config_file/config.txt"
- Accordingly, please rename the corresponding variables as:
os.environ["STEMMUS_SCOPE"] = read_config(stemmus_scope_config_file)["ExeFilePath"]
In cell 2.1,
stsc.initialize(stemmus_scope_config_file)
In cell 2.2.,
mf6 = ModflowApi(modflow_exe_file, working_directory = modflow_working_directory)
mf6.initialize(modflow_config_file)
- To help users in locating the required files, in the introduction section of your notebook, when mentioning the Zenodo entry, please include the relative paths to the necessary items and tell users to copy these files to their own working directory. For example,
modflow_working_directory = ./SSM_examples/ex1/mf6_model_base
-The variable names in all cells of the notebook:
please use clear and readable names in the code. It's better to have a longer, more descriptive name than a short, unclear one. For example, instead of stsc
, consider using stemmus_scope
or stemmus_scope_model
.
-RuntimeError
I'm not sure why a RuntimeError
should be raised during the model finalization process. If there's an issue, finalizing the models should be handled in separate cells. Otherwise, if an error occurs in modflow, for example, it will prevent stemmus_scope from being finalized properly.
-Documentation:
Please add the notebook to the documentation, by adding an item as - "example: coupling modflow and stemmus_scope": notebooks/bmi_MODFLOW_coupling.ipynb
in the BMI
section in mkdocs.yml file.
If something is not clear, please let me know.
Co-authored-by: SarahAlidoost <[email protected]>
Co-authored-by: SarahAlidoost <[email protected]>
Many thanks @SarahAlidoost for the comments and suggestions. I did all of them and updated the notebook. Have a look again please at the notebook and let me know if anything is still required. |
Thanks for addressing the comments. I saw the link to https://flopy.readthedocs.io/en/stable/tutorials.html#modflow-6. I created a model simulation using the code block in https://flopy.readthedocs.io/en/stable/Notebooks/mf6_data_tutorial01.html. However, I couldn't run the coupling notebook with the modflow files because some parameters needed to be adjusted. Can you please add a complete code block to your coupling notebook that creates the same |
@MostafaGomaa93 can you please merge the main branch to your branch in this pull request? So, the merge conflicts are resolved. |
Done |
Quality Gate passedIssues Measures |
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.
@MostafaGomaa93 thanks for adding the notebook that shows the coupling workflow. Nice addition to the documentation 👍 I updated the docker versions and added some tests for new changes to config_io. Ready to merge.
relates #103
🔴 This branch is based on the branch
bmi-groundwater-coupling
in PR #101, should be merged after that.