Skip to content

Add laminar flow to manning - #2544

Merged
visr merged 15 commits into
mainfrom
Add_laminar_flow_to_manning
Sep 9, 2025
Merged

Add laminar flow to manning#2544
visr merged 15 commits into
mainfrom
Add_laminar_flow_to_manning

Conversation

@verheem

@verheem verheem commented Aug 27, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@verheem
verheem marked this pull request as ready for review August 28, 2025 09:06
@verheem
verheem requested a review from visr August 28, 2025 09:07
@visr

visr commented Aug 28, 2025

Copy link
Copy Markdown
Member

This seems like a nice improvement, replacing the current hardcoded threshold for the relaxed root with a dynamic one based on the Reynolds number. We can keep #2471 open in case we also want to add e.g. a DarcyWeisbachResistance node.

@Huite what do you think? Based on testing real models this does not affect performance much, but should be easier to explain.

This still need an update of the ManningResistance docs at https://ribasim.org/reference/node/manning-resistance.html, after "Therefore we use the modified expression"

Comment thread core/src/solve.jl Outdated
@visr visr mentioned this pull request Sep 8, 2025
4 tasks
@Huite

Huite commented Sep 8, 2025

Copy link
Copy Markdown
Contributor

@Huite what do you think? Based on testing real models this does not affect performance much, but should be easier to explain.

Looks good to me! As a sanity check, it might be worthwhile to see for e.g 10 sets of connection parameters (slope, width, etc.) how the new threshold compares to the old.

visr added a commit that referenced this pull request Sep 9, 2025
This is optimistically targeting a release date of tomorrow, and already
includes these assuming the will be in the release.

- [ ] #2564
- [ ] #2562
- [ ] #2544
- [ ] #2542
@visr
visr merged commit 71ea905 into main Sep 9, 2025
19 of 20 checks passed
@visr
visr deleted the Add_laminar_flow_to_manning branch September 9, 2025 14:37
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.

4 participants