Skip to content
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

Use multiprocessing get_context and python 3.12 #249

Merged
merged 3 commits into from
Mar 13, 2024

Conversation

braingram
Copy link
Collaborator

@braingram braingram commented Mar 12, 2024

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 spacetelescope/jwst#8306 can lead to unexpected behavior.
This PR for jwst removes set_start_method (and requires the source branch for this PR for sctal):
spacetelescope/jwst#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

  • added entry in CHANGES.rst (either in Bug Fixes or Changes to API)
  • updated relevant tests
  • updated relevant documentation
  • updated relevant milestone(s)
  • added relevant label(s)

@braingram
Copy link
Collaborator Author

braingram commented Mar 12, 2024

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 spacetelescope/romancal#1128 (with one known issue with tweakreg not related to this PR).

@braingram braingram marked this pull request as ready for review March 12, 2024 18:03
@braingram braingram requested a review from a team as a code owner March 12, 2024 18:03
@braingram braingram requested a review from schlafly March 13, 2024 20:13
@braingram
Copy link
Collaborator Author

@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
Copy link
Collaborator

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.

@braingram braingram merged commit cc054eb into spacetelescope:main Mar 13, 2024
23 of 24 checks passed
@braingram braingram deleted the py312 branch March 13, 2024 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants