🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/160449

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit the bot commands wiki

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 92a138e with merge base 32911ff ():

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@albanD the proximal reason I ended up here is that I want c10d ops to always be defined even if they don't have implementations, but these ops depend on ProcessGroup and other C structs that are torchbind'ed so I can't actually stub these, and then I was like, why don't we just always build and link this in.

It is quite annoying, this makes PyTorch not compilable today with gcc14 (which is the default on Fedora used at Meta).
Now I will have to hunt down the right dependency of distributed that doesn't work and hope there is a working flag to disable it.

Removing this means that the distributed team needs to sign up to have ubn on newer version and dependency upgrades to not prevent PyTorch from being buildable.
Today, these things gets fixed over 6months to a year, depending on when we have bandwitdth and when we can actually do upgrades of dependencies like nccl.

It is quite annoying, this makes PyTorch not compilable today with gcc14 (which is the default on Fedora used at Meta).

Help me understand in more detail what's going on here. Is the problem c10d, or is the problem the NCCL backend for c10d. Because I still allow you to disable NCCL build (and if you like, I can change the meaning of USE_DISTRIBUTED=0 to mean "compile c10d but don't compile any of the distributed backends).

Removing this means that the distributed team needs to sign up to have ubn on newer version and dependency upgrades to not prevent PyTorch from being buildable.

Uhhh... yes?? We should??? Like in what universe is distributed not being buildable not a release UBN. If we think NCCL is old and rickety, I hear we have something coming down the pipe... https://docs.google.com/document/d/1OwrW8QOLw1HJDzuBwTgvKCzPAQsDywcCzwCnwBWs-1g/edit?tab=t.0#heading=h.4zcurf5wxe5n

help me understand in more detail what's going on here

There were two separate incidents here in the last couple months: one is about tensorpipe and one about nccl. The problem is that tensorpipe has been archived for a while, so Nikita had to unarchive it to fix it. On the NCCL side, any upgrade is a big risky change that takes a lot of time to get in (after it is fixed upstream).

Uhhh... yes?? We should??? Like in what universe is distributed not being buildable not a release UBN

The problem here is that it's not a PyTorch release but the work Fedora machine I use which uses a recent gcc. So until we upgrade our CD machines to this, then it's not release blocking. But it's also a self-fullfilling prophecy because we won't upgrade the CD machines if it breaks everything. So it will never be a release UBN because we won't force that to happen.
It is still a big challenge for devs doing new CPython enablement, running on non-devserver machines, non-linux machines, etc.
These are not considered UBN and so these issues are not worked on by the distributed team.

If your goal is to have consistent binding, would an option to move the binding outside of the USE_DISTRIBUTED flag. And make everything single one of them error out if called when the flag is not set?

There were two separate incidents here in the last couple months: one is about tensorpipe and one about nccl. The problem is that tensorpipe has been archived for a while, so Nikita had to unarchive it to fix it. On the NCCL side, any upgrade is a big risky change that takes a lot of time to get in (after it is fixed upstream).

This sounds like problems with distributed backends specifically. So would you be happy if there is just a way to disable all the backends? This way we consistently have bindings for everything, but they are all stubbed out (as the backends are not defined).

Oops I stamped before reading all the comments. Well, I still think we should do this but I didn't mean to steamroll albans concerns. Anyway the plan is not to build nccl/gloo/tensorpipe by default so things should be ok right? (Are there any tool chain support issues in c10d proper?)

So, I think concretely what I will do is make USE_DISTRIBUTED=0 disable all the backends instead of what it used to do.

@ezyang your PR has been successfully reverted.

Starting merge as part of PR stack under #159889