-
Notifications
You must be signed in to change notification settings - Fork 151
PYTHON: new remote storage example #841
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
base: main
Are you sure you want to change the base?
Conversation
👋 Hi tstamler! Thank you for contributing to ai-dynamo/nixl. Your PR reviewers will review your contribution then trigger the CI to test your changes. 🚀 |
Signed-off-by: Timothy Stamler <tstamler@nvidia.com>
11b9d82
to
b86eebc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a couple of comments.
The initiator initiates transfers and can perform both local and remote operations with storage servers. | ||
|
||
### Running as Client | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a line about how to install packages like gds installation/pip install nixl and provide links here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is part of the NIXL repo now, so these instructions are already in the base README
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Picture misses UCX plugin here to show the network interaction. A little too wordy and feels the point of converged storage with NIXL is getting lost in the picture.
Can we simplify by doing the following
- show the server/client components in two nodes (like in the example)
- Add UCX plugin
- Show the md exchange/control path flow
- Add annotations for datapath
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tstamler one more change can we use the same arrow type for storage data transfer too?
Albeit its local it is a data transfer as well.
What?
Introduce a new sample application for remote storage in Python