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

Fix multiprocessing for Process, Feature, Segment #163

Merged
merged 10 commits into from
Mar 26, 2024
Merged

Conversation

hagenw
Copy link
Member

@hagenw hagenw commented Mar 25, 2024

Closes #165

Ensure, that the multiprocessing argument of audinterface.Process, audinterface.Feature, audinterface.Segment does not result in an pickle related error, if set to True. Multiprocessing relies on pickling all involved objects to communicate between processes.
The error was originally reported in audeering/opensmile-python#104.

I added tests for audinterface.Process, audinterface.Feature, audinterface.Segment, to detect the error.
Most tests included already the multiprocessing argument, but it was always set to False.

The reason for the error consisted of two parts:

  1. In Add per execution process_func_args argument #157 we introduced
signature = inspect.signature(process_func)
Process._process_func_signature = signature.parameters

The problem is that Process._process_func_signature cannot be pickled.
To fix this, I changed the code to

signature = inspect.signature(process_func)
Process._process_func_signature = dict(signature.parameters)
  1. When process_func=None is selected, we provide default process functions. Before they were defined on the fly inside the object, which also results in objects that cannot be pickled. This is solved here, by defining the functions outside of the actual objects.

/cc @frankenjoe

@hagenw hagenw force-pushed the test-multiprocessing branch from f193076 to bfd8e34 Compare March 25, 2024 18:52
@hagenw hagenw force-pushed the test-multiprocessing branch 2 times, most recently from 6f92932 to 1e9f056 Compare March 26, 2024 07:57
Copy link

codecov bot commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.0%. Comparing base (78a10d4) to head (8b27d33).

Additional details and impacted files
Files Coverage Δ
audinterface/core/feature.py 100.0% <100.0%> (ø)
audinterface/core/process.py 100.0% <100.0%> (ø)
audinterface/core/process_with_context.py 100.0% <100.0%> (ø)
audinterface/core/segment.py 100.0% <100.0%> (ø)

@hagenw hagenw changed the title TST: test for multiprocessing=True Fix multiprocessing for Process, Feature, Segment Mar 26, 2024
@hagenw hagenw requested a review from maxschmitt March 26, 2024 09:42
Copy link
Contributor

@maxschmitt maxschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks for the fix.

@hagenw hagenw merged commit 5b0e605 into main Mar 26, 2024
16 checks passed
@hagenw hagenw deleted the test-multiprocessing branch March 26, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiprocessing is broken at the moment
2 participants