docs: grammar tweaks #237
docs: grammar tweaks #237
Conversation
docs/source/index.rst
Outdated
| interface is extended by adding the option :code:`cores_per_worker=2` to assign multiple MPI ranks to each function call. | ||
| To create two workers :code:`max_workers=2` each with two cores each requires a total of four CPU cores to be available. | ||
| interface is extended by adding the option :code:`max_workers=2` to assign two MPI ranks to each function call. | ||
| To give each MPI rank two cores, we set :code:`cores_per_worker=2`, which requires a total of four CPU cores to be available. |
There was a problem hiding this comment.
I was guessing here that the meanings of cores_per_worker and max_workers was swapped, but maybe I was wrong. Just wanted to draw attention to it.
There was a problem hiding this comment.
I understand this part is confusing. As the concurrent.futures.Executor interface defines only the max_workers parameter to specify how many functions are executed at the same time, the pympipool.Executor class extends this interface by adding the cores_per_worker parameter. This additional parameter allows to assign multiple cores to the execution of each function. This enables the parallel execution of the of multiple MPI parallel python function.
As an example, starting 3 workers with each 2 cores, would be defined as max_workers=3 and cores_per_worker=2. I hope this makes this section more clear.
There was a problem hiding this comment.
Oh I had forgotten that max_workers was part of the concurrent.futures interface. But it looks like actually that parameter is only defined by the concrete subclasses, and not concurrent.futures.Executor itself? In which case couldn't you have a completely different parameter with a different name and meaning? To be honest I'm still not certain what max_workers and cores_per_worker do, although I think if I tried it out it would make sense. In the example it might be nice to have different numbers for max_workers and cores_per_worker to help distinguish.
There was a problem hiding this comment.
I reverted my changes to this section.
There was a problem hiding this comment.
I am a bit confused, as both concurrent.futures.ThreadPoolExecutor(max_workers=5) and concurrent.futures.ProcessPoolExecutor(max_workers=5) implement the max_workers parameter. For me the max_workers gives the number of python functions which can be executed in parallel. This is extended in the pympipool.Executor by supporting functions which use MPI internally via the mpi4py interface. So to be able to execute 5 functions in parallel each with 2 cores, the pympipool.Executor can be specified with max_workers=5 and cores_per_worker=2 resulting in a total of 10 cores being used.
|
Hi @jameshcorbett , Thank you so much for taking a look at the documentation, I really appreciate your feedback. |
Problem: some of the grammar in the docs could be improved. Move some commas around and add some dashes.
jameshcorbett
force-pushed
the
docs-tweaks
branch
from
37ef506 to
d98e924
Compare
@jan-janssen asked me to look at the documentation. I think it looks good! I was a little confused by the last sentence of the first paragraph (the one that starts "Each of these solutions has their advantages and disadvantages...") so I gave my best attempt to reformulate it. I think it's an important sentence because it's really the sales pitch for
pympipoolrelative to the other libraries described.