Conversation
Decorating a top-level function with @task would replace the function with a Task that references the function. However, the Task would be updated with functools.update_wrapper to look like the function. This included the __name__ and __module__ which essentially clobbered the original function so pickling Task would fail because the reference to the wrapped function did not resolve to the right thing anymore. This was fixed by switching to decorating with functions only (via functools' wraps and partial). I.e., there is not Task object now (well there is but it's just a protocol for what the wrapper functions do). There's some tricky typing and serialization consequences that I tried to leave as many comments for and added serializatin and typing tests.
gpauloski
added
the
enhancement
This raises a deprecation warning on POSIX where the default is fork() which will change in 3.14. It is also unsafe to fork the test process because it is multithreaded. See python/cpython#100228 and https://docs.python.org/3/library/multiprocessing.html#contexts-and-start-methods
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add the
@task()decorator so task functions can be serialized by reference.This was motivated in #121 by TaskVine's serverless mode wanting to refer to task functions by name, something that was previously not possible because functions were wrapped into
Taskclass instances. Now, task functions defined in the top-level of a module can be mutated into aTask(which is now just aProtocoland all tasks are just functions rather than class instances).This is fully backwards compatible. This did require some mildly complex logic to (1) ensure decorated functions can still be called like normal functions outside of the executor, (2) the
Enginecan still accept normal functions (it will internally wrap the function as a task), and (3) to make suretask()can be used as a decorator on a top-level function and to create a new function when theEngineis wrapping a function.A notable side-effect of this change is that the
TaskTransformeris now an argument of a task injected by theEngine. Thus, it is serialized/deserialized along with every method invocation. This was sometimes already the case in the old system if the executor did not cache functions. I expect the impact of this change to be trivial.This change does add a nice feature that the name of a task can be customized if the name of the function associated with the task does no suffice.
Also fixes a few issues in the test suite:
ProcessPoolExecutoron POSIX platforms.Noneis bugged and does not work, so to avoid warnings for already used ports I've set the ports to random.Fixes
@taskdecorator for app task functions #121Type of Change
Testing
All tests pass, new ones added where needed, and tested the cholesky app.
Pull Request Checklist
Please confirm the PR meets the following requirements.
pre-commit(e.g., ruff, mypy, etc.).