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

Refactor initialization Model #562

Merged
merged 14 commits into from
Apr 7, 2025
Merged

Refactor initialization Model #562

merged 14 commits into from
Apr 7, 2025

Conversation

vers-w
Copy link
Collaborator

@vers-w vers-w commented Mar 18, 2025

Issue addressed

Fixes #523

SouthEndMusic and others added 7 commits March 5, 2025 17:47
* Give (non-type) global constants upper case names and fixes issue #535
* Remove parametric type `T` from structs
This is always set at Float64. Supporting parametric precision could be part of a separate PR (if we want to support this), including which structs would need type parameters.

---------

Co-authored-by: Willem van Verseveld <[email protected]>
test/run_sbm.jl passes

Co-authored-by: Bart de Koning <[email protected]>
test/run_sbm_gwf.jl passes
Containing shared land and river parameters, and `Network` data structure. Tests run_sbm and run_sbm_gwf pass.
Make use of shared river and land parameters.
@vers-w vers-w mentioned this pull request Mar 18, 2025
5 tasks
vers-w added 5 commits March 18, 2025 19:07
In some cases the same data from netCDF is read more than once, in the function `ncread` logging is now optional when reading a netCDF variable.
Rename `fraction_to_river` to `flow_fraction_to_river` and `river` to `river_location`.
@vers-w vers-w marked this pull request as ready for review March 20, 2025 15:12
@vers-w vers-w requested a review from JoostBuitink March 20, 2025 15:12
Copy link
Contributor

@JoostBuitink JoostBuitink left a comment

Choose a reason for hiding this comment

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

Great work, thanks for this! I have a couple of (small) comments/notes, but looks good to me otherwise! It would be good to get some more insight on the changed tests in the sediment model (question asked through Teams) before full approval/merge.

Copy link
Contributor

@JoostBuitink JoostBuitink left a comment

Choose a reason for hiding this comment

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

Thanks for the final changes, looks good to me!

@vers-w vers-w merged commit 2daa6e5 into master Apr 7, 2025
8 of 9 checks passed
@vers-w vers-w deleted the refactor_init_model branch April 10, 2025 15:20
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.

Refactor and align Model initialization functions
3 participants