-
Notifications
You must be signed in to change notification settings - Fork 21
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
Use Configurations.jl
for configuration
#519
base: master
Are you sure you want to change the base?
Conversation
Example config print:
|
@option struct Input <: TableOption | ||
path_forcing::String = "." | ||
path_static::String = "." | ||
gauges::String = "wflow_gauges" | ||
gauges_grdc::String = "wflow_gauges_grdc" | ||
ldd::String = "wflow_ldd" | ||
river_location::String = "wflow_river" | ||
subcatchment::String = "wflow_subcatch" | ||
forcing::Vector{String} = String[] | ||
cyclic::Vector{String} = String[] | ||
vertical::InputVertical = InputVertical() | ||
lateral::InputLateral = InputLateral() | ||
end |
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.
Perhaps good to mention here, is that in the input section, we allow users to define their own maps, which can be used to write output (think of different subbasin maps, different types of gauges). So there are a couple of required option (e.g. path_forcing
, path_static
), it is also a very flexible section).
Same holds for the output sections: this can be any parameter (e.g. lateral.river.q and lateral.river.h, but also vertical.satwaterdepth, etc), so I don't think we would want to explicitly mention all the available options here, because that will become quite messy.
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.
I see. I'm not sure such flexibility is possible within this structure, better ask @evetion.
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.
Maybe worth to implement our own structure/methods for this?
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.
I guess at least some of the config structure is static and could use Configurations, and for the flexible part you use a dict?
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, good point, we could indeed use Configurations for the static part.
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, what a huge config you guys have. 😁
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 pay a certain price for flexibility 😉...
Issue addressed
Fixes #243.
Additional Notes (optional)
Probably needs to be coordinated with #518.