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

Fix dommel network #129

Merged
merged 8 commits into from
Aug 20, 2024
Merged

Fix dommel network #129

merged 8 commits into from
Aug 20, 2024

Conversation

DanielTollenaar
Copy link
Collaborator

@DanielTollenaar DanielTollenaar commented Aug 16, 2024

Contains all work for #102, including:

  • NetworkValidator to automated fixing of topological issues
  • Misc new methods for the Ribasim-NL Model class:
    • reverse_edge: reverse an edge and it's from and toproperties
    • remove_node remove a node, possibility to remove connected_edges as well
    • remove_edge: remove an edge, possibility to remove disconnected_nodes as well
  • the modify_model.py script to work from Sweco 2024.6.3 model to 2024.8.0 available at https://deltares.thegood.cloud/f/117827

All results in a topologically correct network that passes the RIBASIM network validator and simulation. Note, all parameters are implausible still:
image

@DanielTollenaar DanielTollenaar self-assigned this Aug 16, 2024
Copy link
Member

@visr visr left a comment

Choose a reason for hiding this comment

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

Good to have this! Left some small comments.

notebooks/de_dommel/modify_model.py Outdated Show resolved Hide resolved
notebooks/de_dommel/modify_model.py Show resolved Hide resolved
src/ribasim_nl/ribasim_nl/model.py Show resolved Hide resolved
src/ribasim_nl/ribasim_nl/network_validator.py Outdated Show resolved Hide resolved
visr added a commit that referenced this pull request Aug 19, 2024
Fixes #122.

At this point we'll work with the already uploaded results of ribasim_lumping and patch those up for use with the current Ribasim version, like for instance done in #129.

That means for now we don't need to update this code to run. I don't delete the code because we probably want to use it later on and may need to look at the implementation. This does mean we cannot `import ribasim_lumping` within the pixi environment.
@visr visr merged commit 139f57a into main Aug 20, 2024
4 checks passed
@visr visr deleted the fix_dommel_network branch August 20, 2024 11:37
@DanielTollenaar DanielTollenaar mentioned this pull request Aug 21, 2024
8 tasks
visr added a commit that referenced this pull request Aug 22, 2024
Fixes #122.

At this point we'll work with the already uploaded results of
ribasim_lumping and patch those up for use with the current Ribasim
version, like for instance done in #129.

That means for now we don't need to update this code to run. I don't
delete the code because we probably want to use it later on and may need
to look at the implementation. This does mean we cannot `import
ribasim_lumping` within the pixi environment.

Also updates dependencies with `pixi update`.
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