8000 build: remove dependency install for nixl by nv-anants · Pull Request #594 · ai-dynamo/dynamo · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

build: remove dependency install for nixl #594

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

Closed
wants to merge 7 commits into from

Conversation

nv-anants
Copy link
Contributor

Overview:

Dependencies are already present as part of cuda-dl-base image

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • closes GitHub issue: #xxx

Dependencies are already present as part of cuda-dl-base image
Copy link
copy-pr-bot bot commented Apr 11, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Signed-off-by: Tim Stamler <tstamler@nvidia.com>
Signed-off-by: Tim Stamler <tstamler@nvidia.com>
@nnshah1
Copy link
Contributor
nnshah1 commented Apr 25, 2025

are we concentrating on this request or this one:

#407

@nv-anants
Copy link
Contributor Author

are we concentrating on this request or this one:

#407

this one. closed 407

@@ -182,6 +73,9 @@ RUN mkdir /opt/dynamo && \
ENV VIRTUAL_ENV=/opt/dynamo/venv
ENV PATH="${VIRTUAL_ENV}/bin:${PATH}"

# Install NIXL Python module
RUN cd /opt/nixl && uv pip install .
Copy link
Contributor

Choose a reason for hiding this comment

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

For ARM support it looks like we need to set gds_path explicitly because it's hard-coded on nixl side atm. This worked for me on a quick test until we can move the arch detection to NIXL side:

# Install NIXL Python module
# TODO: Can NIXL build select gds_path based on arch instead of us?
RUN if [ "$ARCH" = "arm64" ]; then \
      cd /opt/nixl && uv pip install . --config-settings=setup-args="-Dgds_path=/usr/local/cuda/targets/sbsa-linux/"; \
    else \
      cd /opt/nixl && uv pip install .; \
    fi

Copy link
Contributor

Choose a reason for hiding this comment

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

This PR looks great to me otherwise!

@rmccorm4
Copy link
Contributor

I've included and extended this PR with ARM support in this PR: #839

Signed-off-by: Anant Sharma <anants@nvidia.com>
@nv-anants nv-anants closed this Apr 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0