-
-
Notifications
You must be signed in to change notification settings - Fork 370
r.blend: Add nprocs parameter and use it for r.mapcalc #6564
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
base: main
Are you sure you want to change the base?
Conversation
The main step in r.blend is running r.mapcalc which is now multithreaded. The default number of threads is now whatever is available on the machine, so to limit that, r.blend needs nprocs which is passed to r.mapcalc (before that change, r.blend was not taking the advantage of multithreading in r.mapcalc).
|
The new multithreaded behavior of r.mapcalc affects all other modules using it, including addons. That can be an issue especialy when usage of r.mapcalc is parralaleized there in parallel runs... How about adding tha |
I agree, especially when the parallelization in mapcalc is still somewhat experimental. @marisn any opinion on this, since you lead the effort to change the default? |
I can not judge the state of parallel code in r.mapcalc thus can not say if it should be pinned to 1 or not. Current logic is "opt-out of parallel execution" to aid average user who would not go down to arcane settings. Having a default of 1 would mean "opt-in for parallel execution", what would result in most of time parallel code being just a waste of our development time as it is not used. When we look at developers, who should understand better various options, they have to manage nprocs by either hard-coding or exposing to an end-user. Although we can rethink the default (e.g. nprocs <= 8) as CPUs with high number of cores become more popular. |
|
The practical problem is that r.mapcalc is everywhere so there needs to be more work to properly expose the nprocs setting from all the tools like r.blend. Also there are still some unresolved issues with r.mapcalc, so using default 1 in the 8.5 release would perhaps make the transition easier and give us time to resolve these issues, and change the default to higher number later. |
With this change, callers of gs.mapcalc can now use nprocs to set number of processes for the underlying r.mapcalc. This makes it available to a number of Python tools which use gs.mapcalc to call r.mapcalc (as well as to user scripts). The default is set to 1. Specifically, the parameter default is None which is translated to 1 as opposed to using r.mapcalc's default. As a result, r.mapcalc's default is now independent from the default of gs.mapcalc. At this point, it makes all the Python calls using gs.mapcalc behave as before, i.e., one core only, while direct usage of r.mapcalc uses the all-cores default as do the other C tools now. See also OSGeo#6564 (discussion on default for r.mapcalc), OSGeo#5731 (default for nprocs in C), and OSGeo#5742 (r.mapcalc OpenMP version).
With this change, callers of gs.mapcalc can now use nprocs to set number of processes for the underlying r.mapcalc. This makes it available to a number of Python tools which use gs.mapcalc to call r.mapcalc (as well as to user scripts). The default is set to 1. Specifically, the parameter default is None which is translated to 1 as opposed to using r.mapcalc's default. As a result, r.mapcalc's default is now independent from the default of gs.mapcalc. At this point, it makes all the Python calls using gs.mapcalc behave as before, i.e., one core only, while direct usage of r.mapcalc uses the all-cores default as do the other C tools now. Adds nprocs also to the mapcalc_start function with copy-pasted default and doc. See also #6564 (discussion on default for r.mapcalc), #5731 (default for nprocs in C), and #5742 (r.mapcalc OpenMP version).
The main step in r.blend is running r.mapcalc which is now multithreaded. The default number of threads is now whatever is available on the machine, so to limit that, r.blend needs nprocs which is passed to r.mapcalc (before that change, r.blend was not taking the advantage of multithreading in r.mapcalc).
Possible improvement
To the documentation, I wanted to add a note along the lines of
However, I'm not able to obtain the "too many files" error with the current main regardless of the number of cores specified or the number of rows used. (The main branch has now the file descriptor reuse optimization, but high enough nprocs still should trigger the issue, no?)
How relevant is the note is also questionable because all tools using r.mapcalc should have this, because all hide this r.mapcalc behavior in them. There is nothing special about r.blend in particular (it would be without the file descriptor fix).
Additional context
Same fix is needed for a number of tools which are using r.mapcalc.