Skip to content

Conversation

nluehr
Copy link
Contributor

@nluehr nluehr commented Apr 8, 2022

Now that TensorRT is available in the regular CUDA apt repos, the
'machine-learning' repos are no longer needed in the GPU setup instructions.

Now that TensorRT is available in the regular CUDA apt repos, the
'machine-learning' repos are no longer needed in the GPU setup instructions.
Copy link

@kmittman kmittman left a comment

Choose a reason for hiding this comment

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

Thank you for updating these to use cuda-keyring package to enable the CUDA repository.

@8bitmp3
Copy link
Contributor

8bitmp3 commented May 2, 2022

Thanks @nluehr !

@mihaimaruseac @MarkDaoust PTAL, thanks!

@github-actions github-actions bot added the lgtm Community-added approval label May 2, 2022

# Install TensorRT. Requires that libcudnn8 is installed above.
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean to remove the TensorRT installation instructions? Have they changed?

The PR description only notes that TensorRT is available elsewhere, I assume it still needs to be installed?

If this section does need to be removed, then the "software requirements" section will need to be updated to remove the "install tensorrt" entry.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @mihaimaruseac Can you also please check the TensorRT section ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The TensorRT installation has been combined with the rest of the CUDA installation (see the libnvinfer* packages) since all packages are now available from a common apt repo.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for that info - please update the "software requirements" section to remove the "install tensorrt" entry.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead I added a comment noting that libnvinfer optional and needed for TensorRT.
PTAL.

@markmcd
Copy link
Member

markmcd commented May 3, 2022

@8bitmp3 I've taken a look but it looks like I'm not an owner here so can't actually approve, sorry!

mihaimaruseac
mihaimaruseac previously approved these changes May 4, 2022
mihaimaruseac
mihaimaruseac previously approved these changes May 4, 2022
@8bitmp3 8bitmp3 added the ready to pull Start merge process label May 5, 2022
8bitmp3
8bitmp3 previously approved these changes May 5, 2022
@8bitmp3 8bitmp3 added ready to pull Start merge process and removed ready to pull Start merge process labels May 5, 2022
@copybara-service copybara-service bot merged commit a860e49 into tensorflow:master May 5, 2022
@nluehr nluehr deleted the cuda-repo-update branch May 5, 2022 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm Community-added approval ready to pull Start merge process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants