-
Notifications
You must be signed in to change notification settings - Fork 22
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
Resolve overhead Threads.@threads
(> 8 threads)
#261
Merged
Merged
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
Make use of Threads.@Spawn and basesize to have more control on the amount of work per thread. Added this for vertical and kinematic wave routing components.
and removed eachindex when not required
Use only catchment scale stream order to partition river and land domain for parallel running of kinematic wave solution. Different minimum stream order values can be supplied for the river and land domain.
SurfaceFlow is used for both kinematic wave river and overland flow. This is now split into structs SurfaceFlowRiver and SurfaceFlowLand. It simplifies the orginal update function (split into two, logic easier to follow), and dispatching on these types is easier (e.g. for BMI to extract relevant properties as grid type).
use threaded_foreach instead of @threads
For threads <= 8 spawn tasks is used (outperforms Polyester threads in this range), and for threads > 8 Polyester threads is used (spawn tasks slows down in this range).
Reduce run time by applying loopvectorization only for edges above flow threshold and by pre-allocating more variables (river and floodplain).
and stable_timestep calculation is now faster with @tturbo.
and rename variable index of struct FloodPlain
- update model parameters docs because of split of struct SurfaceFlow in river and overland flow structs. - remove field to_river from struct SurfaceFlowRiver (not used as part of river flow).
For Julia version < 1.8 Base.ifelse is not working with Loopvectorization (`LoopVectorization.check_args` on your inputs failed).
JoostBuitink
approved these changes
Jun 8, 2023
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.
Great work! Looks good to me (and some additional testing I did). Only have one minor comment to update the docstring, but other than that, it looks good to go!
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.
The use of
Threads.@threads
has changed to:SBM
concept (<= 8 threads), for more than 8 threads the low overhead threadingPolyester.@batch
is used.Polyester.@batch
for 2D overland flow local inertial routing and the local inertial continuity equation.