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

DaskVine Serverless #3581

Merged

Conversation

BarrySlyDelgado
Copy link
Contributor

@BarrySlyDelgado BarrySlyDelgado commented Nov 29, 2023

Proposed changes

This enables serverless with DaskVine. A new class, FunctionCallDask, creates function calls instead of PythonTasks. These options set with the task_mode option.

Post-change actions

Put an 'x' in the boxes that describe post-change actions that you have done.
The more 'x' ticked, the faster your changes are accepted by maintainers.

  • make test Run local tests prior to pushing.
  • make format Format source code to comply with lint policies. Note that some lint errors can only be resolved manually (e.g., Python)
  • make lint Run lint on source code prior to pushing.
  • Manual Update Did you update the manual to reflect your changes, if appropriate? This action should be done after your changes are approved but not merged.
  • Type Labels Select github labels for the type of this change: bug, enhancement, etc.
  • Product Labels Select github labels for the product affected: TaskVine, Makeflow, etc.
  • PR RTM Mark your PR as ready to merge.

Additional comments

I have tested it on a small scale locally and distributed. There are still some things to fix.

@dthain dthain requested a review from btovar November 30, 2023 13:48
@dthain
Copy link
Member

dthain commented Dec 13, 2023

Please merge or rebase on master.

Also note change needed to #3589 .

Also talk to @tphung3 about the bug he just found in scheduling!

@dthain
Copy link
Member

dthain commented Dec 18, 2023

Taking a quick look, I see that an output file is declared in submit_finalize. Is there a corresponding remove_file in the destructor?

@BarrySlyDelgado
Copy link
Contributor Author

No, there is no remove_file at the moment.

@dthain
Copy link
Member

dthain commented Dec 18, 2023

I'm asking you if it needs one. :)

@BarrySlyDelgado
Copy link
Contributor Author

It can be added to the destructor however, the DaskVine executor will need only to delete the FunctionCall tasks and remove the files once all the tasks that may consume its output are completed.

@dthain
Copy link
Member

dthain commented Jan 10, 2024

@BarrySlyDelgado please rebase or merge on master to pick up the memory leak fix from @JinZhou5042 .

@dthain
Copy link
Member

dthain commented Jan 29, 2024

@BarrySlyDelgado is this the code being used to run HEP applications? If so, rebase on master and let's get this merged.

@dthain
Copy link
Member

dthain commented Jan 30, 2024

Hello?

@BarrySlyDelgado
Copy link
Contributor Author

Yes, this is currently used for testing function calls for the HEP applications.

@dthain
Copy link
Member

dthain commented Feb 2, 2024

I'm ready to merge this once you have pulled in the necessary changes from master.

@btovar btovar merged commit baed3a8 into cooperative-computing-lab:master Feb 19, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants