add reduction_factor to TabulatedRatingCurve #562
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If the rating curve prescribes large Q even when the upstream Basin is nearly empty, this leads to an incorrect water balance.
The commit waiting times are just too annoying
Won't be registered, but good to bump this for the release. Co-authored-by: Hofer-Julian <[email protected]>
This works fine having the file open in QGIS on Windows. We write to another file, and at the end move this file over the destination file. Fixes #563
*Edit: this initial message is outdated, see the discussion below.*
It turned out to be quite easy to get `ForwardDiff.jl` to work for the
Jacobian of `water_balance!`, it was mainly a matter of replacing some
instances of `Float64` with `Number` in function signatures or removing
`Float64` altogether (I did this only where necessary).
See the example below for POC.
```julia
using ForwardDiff
using SQLite
using Ribasim
toml_path = "C:/Users/konin_bt/Ribasim_development/Ribasim/data/basic_transient/basic_transient.toml"
cfg = Config(toml_path)
gpkg_path = input_path(cfg, cfg.geopackage)
db = SQLite.DB(gpkg_path)
p = Ribasim.Parameters(db, cfg)
t = 0.0
state = load_structvector(db, cfg, BasinStateV1)
storage, _ = get_storages_from_levels(p.basin, state.level)
integral = zeros(length(p.pid_control.node_id))
u0 = ComponentVector{Float64}(; storage, integral)
function water_balance(u::ComponentVector)
du = zero(u)
water_balance!(du,u,p,t)
return du
end
jac_AD = ForwardDiff.jacobian(water_balance, u0)
jac_AN = zero(jac_AD)
water_balance_jac!(jac_AN,u0,p,t)
println("AD Jacobian:")
display(jac_AD)
println("\nAN Jacobian:")
display(transpose(jac_AN))
```
output:
```
AD Jacobian:
4×4 Matrix{Float64}:
-3.06216e-12 0.0 0.0 0.0
0.0 -2.25533e-7 0.0 0.0
0.0 2.48017e-8 -3.06216e-12 0.0
0.0 4.96033e-8 0.0 -8.26725e-7
AN Jacobian:
4×4 transpose(::Matrix{Float64}) with eltype Float64:
0.0 0.0 0.0 0.0
0.0 -2.2553e-7 0.0 0.0
0.0 2.48017e-8 0.0 0.0
0.0 4.96033e-8 0.0 -8.26722e-7
```
~~There is a remaining problem where the AD Jacobian contained NaNs
unless I turned on NaN-safe mode as suggested
[here](https://juliadiff.org/ForwardDiff.jl/latest/user/advanced.html#Fixing-NaN/Inf-Issues-1).~~
Regarding the current erroring test; why does `Vector{Float64}` not
satisfy `Vector{Number}`?
ed18bb8 accidentally added new manifests rather than resolving the existing ones.
This should fix the TeamCity libribasim tests. `test_bmi.py` did not
catch it since only getting volume is tested:
`libribasim.get_value_ptr("volume")`.
I'll add those tests later, but first let's unbreak CI.
… of 'Test libribasim - Windows' build configuration were updated
When profiling runs with the default `autodiff=true`, this line was responsible for 35% of the time and almost all allocations: https://github.com/Deltares/Ribasim/blob/b3eb044a722d1655c5465bafe50951b75fe960d6/core/src/solve.jl#L1002 With this PR that drops down to 0%. `connectivity.flow` is a sparse matrix, but the DiffCache does not seem to like sparse matrixes. The `dual_du` field was a dense vector of length n x n x cache_size, and the `get_tmp` call led to further allocations trying to restructure the sparse matrix from the vector. Luckily there is the FixedSizeDiffCache that helps here: https://docs.sciml.ai/PreallocationTools/stable/#FixedSizeDiffCache This retains the sparsity in the dual, and returns a `ReinterpretArray` from `get_tmp` during autodiff. To avoid materializing this reinterpretarray I needed to additionally fill the parent array with zeros rather than the array itself. There is another unrelated performance fix here, and that is to concretely type the Parameter struct, by adding type parameters from its fields. Otherwise you have situations like ```julia struct A a::Vector end ``` where the compiler doesn't know the element type of the Vector, so it can perform less optimizations. The solution: ```julia struct A{T} a::Vector{T} end ``` Finally I consistently added AbstractVector/Matrix argument type annotations to ensure the ReinterpretArray could enter everywhere. And I renamed the functions to formulate flows to `formulate_flow`, to make it easier to separate them from the other `formulate!` methods.
`Dictionary` uses `Indices{I}` as keys, and `Vector{T}` as values. The
Parameters contain both, and therefore it was free to construct a
`Dictionary` in a frequently called function like `get_level`. However
with autodiff, the values could be a ReinterpretArray with Duals instead
of just a Vector. This meant that on Dictionary creation it would
convert the ReinterpretArray to a Vector, leading to many allocations.
This is on top of #581. After
that, this was responsible for 94% of the time spent. With this PR that
goes down to about 2%, leading to a nice little speedup.
---------
Co-authored-by: Hofer-Julian <[email protected]>
Bumps [actions/checkout](https://github.com/actions/checkout) from 3 to 4. <details> <summary>Release notes</summary> <p><em>Sourced from <a href="https://github.com/actions/checkout/releases">actions/checkout's releases</a>.</em></p> <blockquote> <h2>v4.0.0</h2> <h2>What's Changed</h2> <ul> <li>Update default runtime to node20 by <a href="https://github.com/takost"><code>@takost</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1436">actions/checkout#1436</a></li> <li>Support fetching without the --progress option by <a href="https://github.com/simonbaird"><code>@simonbaird</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1067">actions/checkout#1067</a></li> <li>Release 4.0.0 by <a href="https://github.com/takost"><code>@takost</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1447">actions/checkout#1447</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/takost"><code>@takost</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1436">actions/checkout#1436</a></li> <li><a href="https://github.com/simonbaird"><code>@simonbaird</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1067">actions/checkout#1067</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v3...v4.0.0">https://github.com/actions/checkout/compare/v3...v4.0.0</a></p> <h2>v3.6.0</h2> <h2>What's Changed</h2> <ul> <li>Mark test scripts with Bash'isms to be run via Bash by <a href="https://github.com/dscho"><code>@dscho</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1377">actions/checkout#1377</a></li> <li>Add option to fetch tags even if fetch-depth > 0 by <a href="https://github.com/RobertWieczoreck"><code>@RobertWieczoreck</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/579">actions/checkout#579</a></li> <li>Release 3.6.0 by <a href="https://github.com/luketomlinson"><code>@luketomlinson</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1437">actions/checkout#1437</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/RobertWieczoreck"><code>@RobertWieczoreck</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/579">actions/checkout#579</a></li> <li><a href="https://github.com/luketomlinson"><code>@luketomlinson</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1437">actions/checkout#1437</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v3.5.3...v3.6.0">https://github.com/actions/checkout/compare/v3.5.3...v3.6.0</a></p> <h2>v3.5.3</h2> <h2>What's Changed</h2> <ul> <li>Fix: Checkout Issue in self hosted runner due to faulty submodule check-ins by <a href="https://github.com/megamanics"><code>@megamanics</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1196">actions/checkout#1196</a></li> <li>Fix typos found by codespell by <a href="https://github.com/DimitriPapadopoulos"><code>@DimitriPapadopoulos</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1287">actions/checkout#1287</a></li> <li>Add support for sparse checkouts by <a href="https://github.com/dscho"><code>@dscho</code></a> and <a href="https://github.com/dfdez"><code>@dfdez</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1369">actions/checkout#1369</a></li> <li>Release v3.5.3 by <a href="https://github.com/TingluoHuang"><code>@TingluoHuang</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1376">actions/checkout#1376</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/megamanics"><code>@megamanics</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1196">actions/checkout#1196</a></li> <li><a href="https://github.com/DimitriPapadopoulos"><code>@DimitriPapadopoulos</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1287">actions/checkout#1287</a></li> <li><a href="https://github.com/dfdez"><code>@dfdez</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1369">actions/checkout#1369</a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v3...v3.5.3">https://github.com/actions/checkout/compare/v3...v3.5.3</a></p> <h2>v3.5.2</h2> <h2>What's Changed</h2> <ul> <li>Fix: Use correct API url / endpoint in GHES by <a href="https://github.com/fhammerl"><code>@fhammerl</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1289">actions/checkout#1289</a> based on <a href="https://redirect.github.com/actions/checkout/issues/1286">#1286</a> by <a href="https://github.com/1newsr"><code>@1newsr</code></a></li> </ul> <p><strong>Full Changelog</strong>: <a href="https://github.com/actions/checkout/compare/v3.5.1...v3.5.2">https://github.com/actions/checkout/compare/v3.5.1...v3.5.2</a></p> <h2>v3.5.1</h2> <h2>What's Changed</h2> <ul> <li>Improve checkout performance on Windows runners by upgrading <code>@actions/github</code> dependency by <a href="https://github.com/BrettDong"><code>@BrettDong</code></a> in <a href="https://redirect.github.com/actions/checkout/pull/1246">actions/checkout#1246</a></li> </ul> <h2>New Contributors</h2> <ul> <li><a href="https://github.com/BrettDong"><code>@BrettDong</code></a> made their first contribution in <a href="https://redirect.github.com/actions/checkout/pull/1246">actions/checkout#1246</a></li> </ul> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Changelog</summary> <p><em>Sourced from <a href="https://github.com/actions/checkout/blob/main/CHANGELOG.md">actions/checkout's changelog</a>.</em></p> <blockquote> <h1>Changelog</h1> <h2>v4.0.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1067">Support fetching without the --progress option</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1436">Update to node20</a></li> </ul> <h2>v3.6.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1377">Fix: Mark test scripts with Bash'isms to be run via Bash</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/579">Add option to fetch tags even if fetch-depth > 0</a></li> </ul> <h2>v3.5.3</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1196">Fix: Checkout fail in self-hosted runners when faulty submodule are checked-in</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1287">Fix typos found by codespell</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1369">Add support for sparse checkouts</a></li> </ul> <h2>v3.5.2</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1289">Fix api endpoint for GHES</a></li> </ul> <h2>v3.5.1</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1246">Fix slow checkout on Windows</a></li> </ul> <h2>v3.5.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1237">Add new public key for known_hosts</a></li> </ul> <h2>v3.4.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1209">Upgrade codeql actions to v2</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1210">Upgrade dependencies</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1225">Upgrade <code>@actions/io</code></a></li> </ul> <h2>v3.3.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/1045">Implement branch list using callbacks from exec function</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1050">Add in explicit reference to private checkout options</a></li> <li>[Fix comment typos (that got added in <a href="https://redirect.github.com/actions/checkout/issues/770">#770</a>)](<a href="https://redirect.github.com/actions/checkout/pull/1057">actions/checkout#1057</a>)</li> </ul> <h2>v3.2.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/942">Add GitHub Action to perform release</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/967">Fix status badge</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1002">Replace datadog/squid with ubuntu/squid Docker image</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/964">Wrap pipeline commands for submoduleForeach in quotes</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1029">Update <code>@actions/io</code> to 1.1.2</a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/1039">Upgrading version to 3.2.0</a></li> </ul> <h2>v3.1.0</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/939">Use <code>@actions/core</code> <code>saveState</code> and <code>getState</code></a></li> <li><a href="https://redirect.github.com/actions/checkout/pull/922">Add <code>github-server-url</code> input</a></li> </ul> <h2>v3.0.2</h2> <ul> <li><a href="https://redirect.github.com/actions/checkout/pull/770">Add input <code>set-safe-directory</code></a></li> </ul> <h2>v3.0.1</h2> <!-- raw HTML omitted --> </blockquote> <p>... (truncated)</p> </details> <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/actions/checkout/commit/3df4ab11eba7bda6032a0b82a6bb43b11571feac"><code>3df4ab1</code></a> Release 4.0.0 (<a href="https://redirect.github.com/actions/checkout/issues/1447">#1447</a>)</li> <li><a href="https://github.com/actions/checkout/commit/8b5e8b768746b50394015010d25e690bfab9dfbc"><code>8b5e8b7</code></a> Support fetching without the --progress option (<a href="https://redirect.github.com/actions/checkout/issues/1067">#1067</a>)</li> <li><a href="https://github.com/actions/checkout/commit/97a652b80035363df47baee5031ec8670b8878ac"><code>97a652b</code></a> Update default runtime to node20 (<a href="https://redirect.github.com/actions/checkout/issues/1436">#1436</a>)</li> <li>See full diff in <a href="https://github.com/actions/checkout/compare/v3...v4">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
|
replaced by #587 |
visr
added a commit
that referenced
this pull request
Sep 12, 2023
If the rating curve prescribes large Q even when the upstream Basin is nearly empty, this leads to an incorrect water balance. Replaces #562.
visr
added a commit
that referenced
this pull request
Sep 14, 2023
If the rating curve prescribes large Q even when the upstream Basin is nearly empty, this leads to an incorrect water balance. Replaces #562.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the rating curve prescribes large Q even when the upstream Basin is nearly empty, this leads to an incorrect water balance.