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

Memory improvements (3/3): Introduce new runner and executor API #1690

Merged
merged 75 commits into from
Nov 9, 2023

Conversation

cjao
Copy link
Contributor

@cjao cjao commented Jun 7, 2023

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

Closes https://github.com/AgnostiqHQ/covalent-staging/issues/709

@cjao cjao requested review from a team as code owners June 7, 2023 03:02
@cjao cjao changed the title Memory improvements test merge (ignore) Memory improvements test merge Jun 7, 2023
@codecov
Copy link

codecov bot commented Jun 7, 2023

Codecov Report

Merging #1690 (acad32a) into develop (889bf3e) will increase coverage by 3.83%.
Report is 5 commits behind head on develop.
The diff coverage is 95.05%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1690      +/-   ##
===========================================
+ Coverage    80.17%   84.00%   +3.83%     
===========================================
  Files          232      290      +58     
  Lines        10239    14120    +3881     
  Branches       193      193              
===========================================
+ Hits          8209    11862    +3653     
- Misses        1897     2125     +228     
  Partials       133      133              
Flag Coverage Δ
Dispatcher 92.22% <96.73%> (+5.86%) ⬆️
Functional_Tests 51.72% <55.48%> (-0.11%) ⬇️
SDK 78.38% <92.71%> (+3.55%) ⬆️
UI_Backend 85.05% <69.60%> (-3.37%) ⬇️
UI_Frontend 73.43% <ø> (ø)

@cjao cjao force-pushed the os-enhancements-preview-testmerge branch from 409c717 to d3ef226 Compare June 7, 2023 14:09
@cjao cjao force-pushed the os-enhancements-preview-testmerge branch 3 times, most recently from 0bd0220 to feedf10 Compare June 8, 2023 03:07
@cjao cjao marked this pull request as draft June 23, 2023 16:18
@cjao cjao marked this pull request as ready for review June 23, 2023 16:20
@cjao cjao force-pushed the os-enhancements-preview-testmerge branch 2 times, most recently from e299df8 to ba0a78e Compare July 7, 2023 16:06
@cjao cjao force-pushed the os-enhancements-preview-testmerge branch 10 times, most recently from 7c3ce70 to 3d3258e Compare July 11, 2023 23:03
@cjao cjao changed the title Memory improvements test merge Memory improvements (3/3): Introduce new runner and executor API Jul 12, 2023
@cjao cjao marked this pull request as draft July 12, 2023 15:57
@cjao
Copy link
Contributor Author

cjao commented Jul 12, 2023

This PR completes the series on memory usage improvements and builds on #1729 . It introduces a new runner to submit tasks and retrieve their results without loading the task's assets in memory. The runner also operates on task groups, not just individual tasks. To use the new runner, each executor plugin must implement a new API for running task groups. If a task's executor plugin does not opt into the new API, Covalent will attempt to fall back to the existing in-memory runner.

@cjao cjao force-pushed the os-enhancements-preview-testmerge branch 3 times, most recently from 459fd89 to 5125bcf Compare July 13, 2023 10:53
@kessler-frost kessler-frost marked this pull request as ready for review October 20, 2023 16:37
@kessler-frost kessler-frost marked this pull request as draft October 31, 2023 17:21
@kessler-frost
Copy link
Member

Converting to draft to add more tests.

@cjao cjao force-pushed the os-enhancements-preview-testmerge branch from 56eb13d to 6701bb0 Compare November 1, 2023 23:29
@cjao cjao force-pushed the os-enhancements-preview-testmerge branch 4 times, most recently from ecd5ee5 to 4f9d12a Compare November 7, 2023 23:27
Running `covalent start` causes the config file to be
updated from two processes:
* the CLI runner, after `_graceful_start()` returns, and
* the DaskCluster process, which records the cluster state after the
cluster starts up.

Unfortunately, the CLI runner previously wrote out the config it
loaded into memory before starting the dispatcher process. Its
ConfigManager instance was therefore unaware of any config file
updates that might have happened before `_graceful_start()`
returned. If the Dask cluster were to finish starting up and write out
its state before the CLI runner returned from `_graceful_start()`, the
CLI's config file update would obliterate the Dask cluster info.

This patch refreshes the CLI app's ConfigManager instance from the
on-disk config file after `_graceful_start()` and ensures that the
Dask cluster finishes starting before `_graceful_start()` returns.
@cjao cjao force-pushed the os-enhancements-preview-testmerge branch from 4f9d12a to 31f0479 Compare November 7, 2023 23:54
@kessler-frost kessler-frost marked this pull request as ready for review November 9, 2023 10:37
@cjao cjao merged commit 1c5ae4a into develop Nov 9, 2023
15 checks passed
@cjao cjao deleted the os-enhancements-preview-testmerge branch November 9, 2023 14:20
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.

3 participants