-
-
Notifications
You must be signed in to change notification settings - Fork 323
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
r.sim.water: add info to manual, add info about progress #4662
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Anna Petrasova <[email protected]>
Co-authored-by: Anna Petrasova <[email protected]>
This sentence may be confusing "The model is for shallow water flow, where "shallow" is defined by hmax"- Perhaps the paragraph could start with "the model tries to keep water "shallow" with maximum shallow water depth defined by hmax value (default is 0.3m). If hmax is reached then .... here goes the text that explains the increase in diffusion rate and related parameters .... The rest of the text that explains when these parameters may need to be adjusted sounds good to me. |
This new description comes from #4475, maybe @petrasovaa can clarify? I am not sure if the model tries to keep water "shallow". After |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.
Locally tested, looks good to me.
May this be merged, @petrasovaa ? |
raster/r.sim/r.sim.sediment/main.c
Outdated
@@ -286,7 +286,9 @@ int main(int argc, char *argv[]) | |||
parm.threads->answer = NUM_THREADS; | |||
parm.threads->required = NO; | |||
parm.threads->description = | |||
_("Number of threads which will be used for parallel compute"); | |||
_("Number of threads which will be used for parallel computation. " | |||
"Increasing the number of threads does not really speed up " |
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.
Please remove this from here, maybe clarify in documentation or add benchmark there. Based on my experience (with r.sim.water, I don't know about r.sim.sediment) a few cores provide somespeed up, although I agree more than 4 cores probably not.
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.
Done with bbf9117
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.
Could you elaborate on this? In your experience, there is no speedup at all? Because that's what the comment suggests. For sure it doesn't scale well, but at least for 4 threads the authors report 3x speedup.
r.sim.water
is a module with a number of important but not well documented parameters. This PR adds more information about some parameters to the manual.This PR tries to add info requested in #4475 to the manual