-
Notifications
You must be signed in to change notification settings - Fork 64
Added doc for nvdec #335
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
Added doc for nvdec #335
Conversation
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.
Great tutorial! It's great to see that the UX for installing gpu-compatible wheels is fairly straightforward. I left a few comments below.
Doc job is failing with a minor build issue: https://github.com/pytorch/torchcodec/actions/runs/11734305974/job/32690104327?pr=335
As a result I haven't been able to look at the rendered docs, please let me do another round before landing :)
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.
Great tutorial! It's great to see that the UX for installing gpu-compatible wheels is fairly straightforward. I left a few comments below.
Doc job is failing with a minor build issue: https://github.com/pytorch/torchcodec/actions/runs/11734305974/job/32690104327?pr=335
As a result I haven't been able to look at the rendered docs, please let me do another round before landing :)
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.
Some more comments, I haven't been able to check the rendered docs yet
run: | | ||
cd docs | ||
python -m pip install -r requirements.txt | ||
${CONDA_RUN} python -m pip install -r requirements.txt |
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.
Do we understand why we need CONDA_RUN 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.
Everything is installed in a conda env. Without CONDA_RUN the pip
that's used is the outside pip
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.
Thanks for the review
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.
Thank you @ahmadsharif1 !
first_cpu_frame = cpu_frames[0].data.to("cpu") | ||
first_cuda_frame = cuda_frames[0].data.to("cpu") | ||
frames_equal = torch.equal(first_cpu_frame, first_cuda_frame) | ||
print(f"{frames_equal=}") |
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.
Nit: Instead of this binary indicator, we may want to print max abs diff and mean abs diff? We could even do it across all frames.
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.
Result screenshot: