glencoesoftware / raw2ometiff

Raw format to OME-TIFF converter
GNU General Public License v2.0
46 stars 20 forks source link

Use a single ThreadPoolExecutor and add a getter #118

Closed melissalinkert closed 9 months ago

melissalinkert commented 9 months ago

Similar to https://github.com/glencoesoftware/bioformats2raw/pull/230, this adds a getter for the ThreadPoolExecutor. The executor usage is also modified so that a single ThreadPoolExecutor is used for the whole conversion, instead of one per resolution.

This approach is consistent with what bioformats2raw does already, but it's worth double-checking a conversion with more than 1 worker to verify that output OME-TIFF is still valid.

DavidStirling commented 9 months ago

The output TIF files appear to be fine regardless of worker count, so that's working correctly.

One issue I've noticed, which may also impact glencoesoftware/bioformats2raw#230 - since we now have a single executor, you're no longer able to run a converter object more than once (as shutdown() gets called on the executor). Any subsequent attempts to run a conversion will instead hang since the pool is no longer processing jobs.

If that's desired behaviour we may still want to check whether the pool is shutdown before trying to run, otherwise another means of waiting for all jobs to finish may be worth investigating. I could imagine that people may want to re-run a conversion that failed/crashed without having to blow away the entire converter object.

melissalinkert commented 9 months ago

Closing as discussed just now with @DavidStirling, since this isn't especially helpful. We'll plan a more organized effort around this topic for the next release.