search

spacetelescope / stcal

https://stcal.readthedocs.io/en/latest/
Other
10 stars 32 forks source link

Use multiprocessing `get_context` and python 3.12 #249

Closed braingram closed 7 months ago

braingram commented 7 months ago

Python 3.12 added a warning when the fork method (default of linux) was used for multiprocessing if a process contains threads:

    /opt/hostedtoolcache/Python/3.12.2/x64/lib/python3.12/multiprocessing/popen_fork.py:66: DeprecationWarning: This process (pid=2229) is multi-threaded, use of fork() may lead to deadlocks in the child.

This warning can be seen in the logs for one of the devdeps jobs that uses python 3.12: https://github.com/spacetelescope/stcal/actions/runs/8110832059/job/22168896740#step:10:169

This PR switches the method to forkserver using a get_context (to only effect the calls on the context).

This PR will allow jwst to remove the global calls to set_start_method which as noted in https://github.com/spacetelescope/jwst/issues/8306 can lead to unexpected behavior. This PR for jwst removes set_start_method (and requires the source branch for this PR for sctal): https://github.com/spacetelescope/jwst/pull/8343 The python 3.12 run shows no DeprecationWarning mentioning fork: https://github.com/spacetelescope/jwst/actions/runs/8253314996/job/22574879663?pr=8343

Finally this PR adds a non-devdeps python 3.12 test job to the CI (which illustrates that the above changes remove the warning from the test suite).

Checklist

braingram commented 7 months ago

Romancal failures are unrelated and also occurring on romancal main: https://github.com/spacetelescope/romancal/actions/runs/8252821337/job/22573282790

jwst regtests all passed: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1305/

romancal regtests ran: https://plwishmaster.stsci.edu:8081/job/RT/job/Roman-Developers-Pull-Requests/649/ all failures are unrelated and appear mostly caused by https://github.com/spacetelescope/romancal/pull/1128 (with one known issue with tweakreg not related to this PR).

braingram commented 7 months ago

@schlafly I requested you as the second reviewer. I did this because I assumed this would be a slight change for romancal (which doesn't at the moment set the multiprocessing mode). However looking at the romancal run logs I'm now thinking that the code changed here is not used by romancal. Would you weight in from the romancal perspective? Thanks!

schlafly commented 7 months ago

We technically have an option to use ols / jump but are not actively using it. This looks pretty innocuous and we shouldn't stand in the way. Thumbs up.