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

Better task manager #65

Merged
merged 28 commits into from
May 7, 2024
Merged

Better task manager #65

merged 28 commits into from
May 7, 2024

Conversation

idocx
Copy link
Member

@idocx idocx commented May 4, 2024

Summary

Include a summary of major changes in bullet points:

  • Feature 1: Make the task manager into task manager (submit new tasks and cancel running tasks) and resource manager (handling resource requests)
  • Feature 2: Add more integration tests to alabos. The unit test will also test the user input module and canceling module to make sure they will work as expected.
  • Fix 1: Fix the unpredictable status caused by canceling function. Previously, there can be status conflict if the status is canceled in WAITING or INITIATED status. Now, the canceling can only happen if the task is in RUNNING or REQUESTING_RESOURCES status.
  • Fix 2: Add a finishing task to make sure the samples not unreleased until the task is fully done. The major changes happen in the task_actor.py. Where the task will be first set to FINISHING state. Until all the procedure have been done, it will be set to corresponding ending status like COMPLETED, CANCELED, etc.
  • Fix 3: Fix the load_definition function. Previously, it is easy to import the alabos configuration folder multiple times. Now we add a __imported_modules__ global variable to keep track of all the imported modules. If the package has been imported, it will be returned directly.
  • Fix 4: Fix the test_launch unittest. Previously, it runs to start a virtual alabos server and submit one experiment. But it will not check the result (e.g. tracking the submitted experiment status). In this way, we can guarantee the whole module works as expected.
  • Fix 5: Fix pyproject.toml dependencies. Previously, there were a lot of duplicated dependencies in the project toml file. They are now removed and the existing dependencies have been updated to the right version.

Additional dependencies introduced (if any)

No additional dependencies are introduced.

Checklist

Work-in-progress pull requests are encouraged, but please put [WIP] in the pull request
title.

Before a pull request can be merged, the following items must be checked:

  • Code is in the standard Python style.
    The easiest way to handle this is to run the following in the correct sequence on
    your local machine. Start with running black on your new code. This will
    automatically reformat your code to PEP8 conventions and removes most issues. Then run
    pycodestyle, followed by flake8.
  • Docstrings have been added in theNumpy docstring format.
    Run pydocstyle on your code.
  • Type annotations are highly encouraged. Run mypy to
    type check your code.
  • Tests have been added for any new functionality or bug fixes.
  • All linting and tests pass.

Note that the CI system will run all the above checks. But it will be much more
efficient if you already fix most errors prior to submitting the PR. It is highly
recommended that you use the pre-commit hook provided in the repository. Simply
cp pre-commit .git/hooks and a check will be run prior to allowing commits.

@idocx idocx self-assigned this May 4, 2024
@idocx idocx requested a review from odartsi May 4, 2024 02:06
@idocx idocx changed the title Better task manager [WIP] Better task manager May 4, 2024
@idocx idocx added the bug not't working as it should be label May 4, 2024
@idocx idocx marked this pull request as ready for review May 5, 2024 22:11
@idocx idocx changed the title [WIP] Better task manager Better task manager May 5, 2024
@idocx idocx marked this pull request as draft May 6, 2024 04:53
@idocx idocx changed the title Better task manager [WIP] Better task manager May 6, 2024
Copy link
Contributor

@odartsi odartsi left a comment

Choose a reason for hiding this comment

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

@idocx , here is my review/questions as well as the tests/test_launch.py that fails.
Thanks

alab_management/experiment_manager.py Show resolved Hide resolved
alab_management/scripts/launch_lab.py Show resolved Hide resolved
pyproject.toml Show resolved Hide resolved
tests/fake_lab/tasks/heating.py Show resolved Hide resolved
tests/fake_lab/tasks/infinite_task.py Show resolved Hide resolved
tests/test_lab_view.py Show resolved Hide resolved
tests/test_launch.py Outdated Show resolved Hide resolved
tests/test_sample_view.py Show resolved Hide resolved
Copy link
Contributor

odartsi commented May 6, 2024

my review is ready

@idocx idocx changed the title [WIP] Better task manager Better task manager May 7, 2024
@idocx idocx requested a review from odartsi May 7, 2024 00:08
@idocx idocx marked this pull request as ready for review May 7, 2024 00:44
@idocx idocx marked this pull request as draft May 7, 2024 05:55
In update_canceling_progress, the original_progress should be enum instead of the raw string.
@idocx
Copy link
Member Author

idocx commented May 7, 2024

@odartsi Thank you so much for your careful reviewing! I have updated the repo according to your comments and fixed some bugs in task_manager.

@idocx idocx marked this pull request as ready for review May 7, 2024 06:02
In update_canceling_progress, the original_progress should be enum instead of the raw string.
@idocx idocx mentioned this pull request May 7, 2024
5 tasks
@odartsi odartsi merged commit fc5bf31 into main May 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug not't working as it should be
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants