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

Vine: add library context implementation #3900

Merged
merged 18 commits into from
Nov 6, 2024

Conversation

tphung3
Copy link
Contributor

@tphung3 tphung3 commented Aug 1, 2024

Proposed Changes

This PR adds the mechanisms needed so that FunctionCalls can pull data hosted in a library on a remote node.

Merge Checklist

The following items must be completed before PRs can be merge.
Check these off to verify you have completed all steps.

  • 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 Update the manual to reflect user-visible changes.
  • Type Labels Select a github label for the type: bugfix, enhancement, etc.
  • Product Labels Select a github label for the product: TaskVine, Makeflow, etc.
  • PR RTM Mark your PR as ready to merge.

@dthain
Copy link
Member

dthain commented Oct 15, 2024

Thanh, what's the status on this one?

@tphung3
Copy link
Contributor Author

tphung3 commented Oct 15, 2024

This is on hold at the moment. I'm working on whether to add the abstraction of context for task groups.

@tphung3 tphung3 self-assigned this Oct 31, 2024
@tphung3 tphung3 changed the title WIP: adding context implementation and working on manager side and library side Vine: add library context implementation Oct 31, 2024
@tphung3 tphung3 requested a review from dthain October 31, 2024 18:47
doc/manuals/taskvine/index.md Outdated Show resolved Hide resolved
# in case of FutureFunctionCall tasks
new_args = []
for arg in args:
if isinstance(arg, dict) and "VineFutureFile" in arg:
Copy link
Member

Choose a reason for hiding this comment

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

This smells like trouble to me. Why does the worker need to know if this is a future file as opposed to the creation of a regular file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this belongs to the future function calls and I don't modify that code except left-indent that code block in. @JinZhou5042 I see that you added this line in #3670. Can you shed some insights please?

taskvine/src/manager/taskvine.h Show resolved Hide resolved
taskvine/src/manager/vine_manager.c Outdated Show resolved Hide resolved
taskvine/src/manager/vine_task.c Outdated Show resolved Hide resolved
taskvine/src/manager/vine_task.h Outdated Show resolved Hide resolved
@tphung3
Copy link
Contributor Author

tphung3 commented Nov 1, 2024

@dthain the new change send a string as exec_mode from Python to C code, and C code converts the string into an enum to use within the C codebase.

@dthain
Copy link
Member

dthain commented Nov 4, 2024

RTM?

@tphung3
Copy link
Contributor Author

tphung3 commented Nov 5, 2024

Tests for special functions (lambda, dynamically executed code) are added with some fix to the PR. This PR is RTM @dthain.

@dthain dthain merged commit 017d556 into cooperative-computing-lab:master Nov 6, 2024
10 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.

4 participants