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.

Sign up for GitHub

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

Jump to bottom

ENH: Scheduler fixes and enhancements with new scheduler integration tests for all supported schedulers #33

Merged
merged 6 commits into from
Jun 21, 2024
Merged

ENH: Scheduler fixes and enhancements with new scheduler integration tests for all supported schedulers #33

merged 6 commits into from
Jun 21, 2024

Conversation

cpelley
Copy link
Collaborator

@cpelley cpelley commented Jun 18, 2024 •
edited
Loading

  • Added integration testing for the schedulers.
    • Testing 'single-threaded' (dask).
    • Testing 'processes' (dask).
    • Testing 'distributed' (dask).
    • Testing 'multiprocessing' (in-house).
    • Ray not yet tested as we don't have an environment with it in (TODO - not high priority).
  • Fixed 'distributed' dask scheduler usage.
    • Switching from 'Node' as dask graph keys to strings (unique string names denoting node identities using dask tokenise). Dask when utilising the distributed scheduler, accepts only tuples, bytes, int, float and str (see dask.core.validate_key).
  • Enabled passing data in memory between nodes for the in-house multiprocessing scheduler.
    • Handles data lifetime (automatically throwing it away when it is not needed anymore).

Issues

Note

Just to raise awareness as the first dagrunner PR under review - since this branch makes changes that impact the reference documentation, an auto reference documentation build commit will be pushed accordingly thanks to the dagrunner action workflow.

image

@cpelley
TEST: end-to-end integration test
2e8340c 
- Demonstrated working 'processes'.
- Demonstrated working 'single-threaded'.
- Added SaveJson plugin.
- Temporarily disabled logger.
@cpelley
Fixed distributed scheduler
1aee7c0
@cpelley
Basic working multiprocessing passing data between nodes
d9849d3
@cpelley cpelley self-assigned this Jun 18, 2024
@cpelley cpelley changed the title Filling test gaps and fixing schedulers Scheduler integration tests and scheduler fixes Jun 18, 2024
@cpelley cpelley changed the title Scheduler integration tests and scheduler fixes Integration tests for supported schedulers and scheduler fixes Jun 18, 2024
@cpelley cpelley added documentation Improvements or additions to documentation enhancement New feature or request test labels Jun 18, 2024
@cpelley cpelley changed the title Integration tests for supported schedulers and scheduler fixes Integration tests for supported schedulers and scheduler fixes and enhancements Jun 18, 2024
@cpelley cpelley marked this pull request as ready for review June 18, 2024 23:42
@cpelley cpelley changed the title Integration tests for supported schedulers and scheduler fixes and enhancements Integration tests for supported schedulers and applied scheduler fixes and enhancements Jun 18, 2024
@cpelley cpelley changed the title Integration tests for supported schedulers and applied scheduler fixes and enhancements ENH: Integration tests for supported schedulers and applied scheduler fixes and enhancements Jun 18, 2024
@cpelley cpelley changed the title ENH: Integration tests for supported schedulers and applied scheduler fixes and enhancements ENH: Scheduler fixes and enhancements with new scheduler integration tests for all supported schedulers Jun 18, 2024
@cpelley cpelley merged commit c1cf31c into main Jun 21, 2024
2 checks passed
@cpelley cpelley deleted the TEST_INTEGRATION branch June 21, 2024 08:48
mo-robert-purvis
mo-robert-purvis reviewed Jun 21, 2024
View reviewed changes
Copy link
Contributor

@mo-robert-purvis mo-robert-purvis 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mo-robert-purvis mo-robert-purvis mo-robert-purvis left review comments

@SamGriffithsMO SamGriffithsMO SamGriffithsMO approved these changes

@PaulAbernethy PaulAbernethy Awaiting requested review from PaulAbernethy

Assignees

@cpelley cpelley

Labels
documentation Improvements or additions to documentation enhancement New feature or request test
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test and verify in-house multiprocessing scheduler
4 participants
@cpelley @PaulAbernethy @mo-robert-purvis @SamGriffithsMO