Skip to content

Decrease relaxed root coefficient to 1e-5#2136

Merged
SouthEndMusic merged 6 commits intomainfrom
relaxed-root
Mar 11, 2025
Merged

Decrease relaxed root coefficient to 1e-5#2136
SouthEndMusic merged 6 commits intomainfrom
relaxed-root

Conversation

@visr
Copy link
Copy Markdown
Member

@visr visr commented Mar 6, 2025

@ngoorden
Copy link
Copy Markdown

ngoorden commented Mar 10, 2025

We can't commit to this branch. We recommend to use 1e-5 so the effect of relaxed_root is minimal on outcomes.

@visr
Copy link
Copy Markdown
Member Author

visr commented Mar 10, 2025

Lowered it to 1e-5.

@visr visr changed the title Decrease relaxed root coefficient to 1e-4 Decrease relaxed root coefficient to 1e-5 Mar 10, 2025
@visr visr requested a review from SouthEndMusic March 10, 2025 11:51
@visr
Copy link
Copy Markdown
Member Author

visr commented Mar 10, 2025 •
edited
Loading

The reason @ngoorden recommends 1e-5 meter/meter is because these low slopes are not common in flowing water. With 1e-3 many situations in the Netherlands were effectively not in our Manning but relaxed root domain, leading to underestimation of flows.

The oscillations from #1743 are not reintroduced by this lower threshold, I made similar plots to confirm. Some changes in the state due to this change are to be expected.

@visr visr marked this pull request as ready for review March 10, 2025 11:57
@visr visr mentioned this pull request Mar 10, 2025
Open
@visr
Slow down benchmark
65785c0 
HWS currently bottlenecks on ManningResistance, and true Manning behavior is more expensive than the relaxed root domain.
@visr visr mentioned this pull request Mar 11, 2025
Merged
@SouthEndMusic SouthEndMusic merged commit d18dd5c into main Mar 11, 2025
27 of 29 checks passed
@SouthEndMusic SouthEndMusic deleted the relaxed-root branch March 11, 2025 10:26
visr added a commit that referenced this pull request Mar 11, 2025
@visr
Write changelog for release (#2144)
5ebfc3c 
Already adds a note on #2136 assuming something like that will be
merged.

- Includes the `snake_case` part of #2114.
- Adds an upper bound on Pandera in pyproject.toml as well (pixi.toml
already has it)
- Fixes #2141 by adding link_id to the error message.
This was referenced Mar 12, 2025
Merged
Closed
Closed
@visr visr mentioned this pull request Jun 5, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SouthEndMusic SouthEndMusic SouthEndMusic approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@visr @ngoorden @SouthEndMusic