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

support call deps and bash deps #726

Merged
merged 37 commits into from
Jun 28, 2022
Merged

support call deps and bash deps #726

merged 37 commits into from
Jun 28, 2022

Conversation

cjao
Copy link
Contributor

@cjao cjao commented Jun 25, 2022

  • I have added the tests to cover my changes.
  • I have updated the documentation and CHANGELOG accordingly.
  • I have read the CONTRIBUTING document.

Closes #676 and #677.

This implementation deviates slightly from the design doc. The Base Dep becomes essentially a CallDep, and the other deps essentially reduce to a CallDep with a dep-specific function.

Subclasses are now implemented by defining a custom python function to apply the dependencies, and passing that function and
associated data to the Base Deps constructor. The Base Deps serializes the function and arguments into
TransportableObjects, and apply() simply returns a tuple (serialized_fn, serialized_args, serialized_kwargs).

The executor accepts a list of such tuples to be deserialized and invoked before and after the main function.

This design essentially ships the previous apply() function to the executor without shipping the entire Deps object.

cjao added 30 commits June 24, 2022 06:58
Covalent's executors currently deserialize the callable in the server
process. We introduce a light wrapper function which deserializes and
invokes the callable and send the wrapper function to the Dask cluster
or remote computer. This has two advantages:

* Since the Covalent dispatcher never touches the deserialized
callable, it doesn't need any of its dependencies.

* When the the remote executor deserializes the wrapper function, the
core callable remains serialized. This allows the wrapper to perform
preparatory steps (such as running shell commands to install deps)
before deserializing the callable.
This way, each executor can decide how to treat each kind of dep. For
instance, a "dask" or "local" executor could ignore ModuleDeps.

Also change Electron signature to accept a `deps` dictionary, e.g.

@ct.electron(deps={"bash": BashDeps(...), "pip": PipDeps(...)})
This allows us to add new supported dep types without modifying each
executor. On the downside, users may encounter unexpected behavior if
they specify a dep type that's incompatible with the executor.

Revisit executor-specific handling of different dep types in another
issue.
Easier for autocompletion
Functions and args are immediately converted to TransportableObjects
and only deserialized immediately before execution.
@cjao cjao changed the title 676 support call before support call_before and call_after Jun 25, 2022
@codecov
Copy link

codecov bot commented Jun 25, 2022

Codecov Report

Merging #726 (288b398) into develop (9c1d195) will decrease coverage by 0.09%.
The diff coverage is 72.22%.

@@             Coverage Diff             @@
##           develop     #726      +/-   ##
===========================================
- Coverage    73.59%   73.49%   -0.10%     
===========================================
  Files           40       43       +3     
  Lines         2060     2177     +117     
===========================================
+ Hits          1516     1600      +84     
- Misses         544      577      +33     
Impacted Files Coverage Δ
covalent/executor/base.py 35.46% <6.25%> (-3.74%) ⬇️
covalent/executor/executor_plugins/dask.py 63.15% <20.00%> (-3.51%) ⬇️
covalent/_workflow/lattice.py 60.83% <63.63%> (+0.23%) ⬆️
covalent/executor/executor_plugins/local.py 83.33% <76.92%> (-8.67%) ⬇️
covalent_dispatcher/_db/dispatchdb.py 93.39% <77.77%> (-1.45%) ⬇️
covalent/_workflow/depsbash.py 84.61% <84.61%> (ø)
covalent/_workflow/electron.py 78.40% <91.66%> (+0.83%) ⬆️
covalent_dispatcher/_core/execution.py 73.00% <91.66%> (+1.48%) ⬆️
covalent/__init__.py 100.00% <100.00%> (ø)
covalent/_shared_files/defaults.py 100.00% <100.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c1d195...288b398. Read the comment docs.

The idea is that all deps eventually reduce to `call_before` and
`call_after`.
@cjao cjao force-pushed the 676-support-call-before branch from 60d0fb7 to 591afca Compare June 26, 2022 00:59
@cjao cjao changed the title support call_before and call_after support call deps and bash deps Jun 26, 2022
@cjao cjao mentioned this pull request Jun 26, 2022
3 tasks
cjao added 2 commits June 26, 2022 11:26
This change should allow all Deps to be implemented in a unified way.

Subclasses are now implemented by defining a custom python
function to apply the dependencies, and passing that function and
associated data to the Base Deps constructor.

The Base Deps serializes the function and arguments into
TransportableObjects, and `apply()` simply returns a tuple of
TransportableObjects:

`(serialized_fn, serialized_args, serialized_kwargs)`
@cjao cjao marked this pull request as ready for review June 27, 2022 11:32
@cjao cjao requested a review from a team as a code owner June 27, 2022 11:32
@scottwn scottwn enabled auto-merge (squash) June 28, 2022 15:46
@scottwn scottwn merged commit 3017837 into develop Jun 28, 2022
@scottwn scottwn deleted the 676-support-call-before branch June 28, 2022 15:59
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.

Implement call_before & call_after hooks for electrons
2 participants