attempt to fix CPU-only bug.

[erip]

Fixes #1539

I don't have a great way to test this, so maybe just good review until we can get GPU in CI? 😓

[erip]

I think there may be some things to address here regarding which channels are added depending on CU_VERSION...

[codecov]

Codecov Report

Merging #1614 (6caaf80) into main (8808e7e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1614   +/-   ##
=======================================
  Coverage   85.33%   85.33%           
=======================================
  Files          58       58           
  Lines        2496     2496           
=======================================
  Hits         2130     2130           
  Misses        366      366           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8808e7e...6caaf80. Read the comment docs.

[parmeet]

Fixes #1539

Hi @erip. Thanks for proposing this PR. Could you please share more details on if these changes are inspired from other domain libs? Unfortunately, I have limited understanding on this.

@malfet, @seemethere Wondering if these changes would indeed resolve the issue? I also see comment here from @malfet
#1520 (comment), but I am not sure which changes exactly caused the issue.

[erip]

Yes, I'm trying to follow the packaging impls from vision and audio. These changes basically add support for newer CUDA toolkits and add these toolkits to the list of reqs as needed.

[parmeet]

Yes, I'm trying to follow the packaging impls from vision and audio. These changes basically add support for newer CUDA toolkits and add these toolkits to the list of reqs as needed.

I see, I may be wrong here but since torchtext does not depend on GPU, so I am not sure if we necessarily need these dependencies. Moreover we don't have the issue #1539 for the stable release packages which does not have these packaging dependencies either. As per the comment here #1520 (comment) I am still trying to understand what changes have triggered the issue. @mthrok do you have some insights on issue #1539?

[erip]

I also may be wrong, but my understanding is that it adds them conditionally: if CU_VERSION is set, it will populate the dependencies correctly. If not, it will fall back to cpu. It's a bit unclear and I'd definitely love someone who has a better understanding to chime in.

I also think there's some room for this to become a more standardized bit of infra among the various domain libs, but maybe we can address the immediate pain before the latent pain. 😄

[Nayef211]

@erip @parmeet should we go ahead and close this since the issue was fixed by reverting #1619?

[erip]

I'm ok with closing it. Do we have a sense of what broke?

[parmeet]

I'm ok with closing it. Do we have a sense of what broke?

We fixed it just by reverting this PR #1520. Presumably the introduction of Pytorch-mutex may have caused the issue. But there are few other changes in packaging so cannot really say for sure. Interestingly the version conflict didn't happen when the PR got reverted.

[parmeet]

@erip @parmeet should we go ahead and close this since the issue was fixed by reverting #1619?

yupp I think it is safe to close it for now.