pyiron / pympipool

up-scale python functions for high performance computing
https://pympipool.readthedocs.io
BSD 3-Clause "New" or "Revised" License
11 stars 3 forks source link

Signature no longer conforms to Process/ThreadPoolExecutor #328

Closed liamhuber closed 1 week ago

liamhuber commented 1 week ago

Both ProcessPoolExecutor and ThreadPoolExecutor take the init argument max_workers -- pympipool did too until recently, but now this is max_cores.

Are these actually fundamentally different? From my understanding these really are the same parameter, and while I feel that pympipool does indeed use the better name here, I would like to go back to max_workers for the sake of matching the standard library interface. Is that possible? The change seems to have been a tangential bit in a larger commit focused on something else.

jan-janssen commented 1 week ago

The issue is that we now allow submitting functions with different resource requirements, so one worker might use one core and another might use two cores and so on. https://pympipool.readthedocs.io/en/latest/examples.html#resource-assignment It is not the number of workers that is limited but rather the number of cores as the CPU has a limited number of processes it can execute per clock cycle. Still I see the same issue that it is inconsistent with the standard library. I could live with adding a max_workers parameter which internally sets the number of max_cores for backwards compatibility with the warning in the Docstring that it is inconsistent.

liamhuber commented 1 week ago

Yeah, that sounds good. Anything that prevents hard errors when swapping out pympipool.Executor for a standard concurrent.futures executor in simple examples would work fine for me.