-
Notifications
You must be signed in to change notification settings - Fork 780
[PyTorch] Fix NCCL communicator init in cuSOLVERMp context creation #3240
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,33 +127,59 @@ class CusolverMpCtx: | |
| """cuSolverMp context for Newton-Schulz matrix orthogonalization. | ||
|
|
||
| Context creation is expensive; create once and reuse across multiple | ||
| :func:`newton_schulz` calls. Call :meth:`destroy` when done. | ||
| :func:`newton_schulz` calls. Creation is collective over ``group`` and | ||
| must be called by every group member on its intended CUDA device. Call | ||
| :meth:`destroy` before destroying ``group``. | ||
| """ | ||
|
|
||
| def __init__(self, group: dist.ProcessGroup) -> None: | ||
| # The cuSolverMp grid borrows the ProcessGroupNCCL communicator. Keep | ||
| # the group alive until the native context has released the grid. | ||
| self._ptr: Optional[int] = None | ||
| self._group: Optional[dist.ProcessGroup] = group | ||
|
|
||
| if not dist.is_initialized(): | ||
| raise RuntimeError( | ||
| "torch.distributed must be initialized before creating CusolverMpCtx" | ||
| ) | ||
|
|
||
| self.nranks = dist.get_world_size(group) | ||
| self._ptr = tex.cusolvermp_ctx_create( | ||
| _get_nccl_comm_ptr(group), dist.get_world_size(group), dist.get_rank(group) | ||
| ) | ||
| self.rank = dist.get_rank(group) | ||
| if self.rank < 0: | ||
| raise RuntimeError("The current process is not a member of the supplied process group") | ||
|
|
||
| comm_ptr = _get_nccl_comm_ptr(group) | ||
| self._ptr = tex.cusolvermp_ctx_create(comm_ptr, self.nranks, self.rank) | ||
|
|
||
| def destroy(self) -> None: | ||
| """Destroy the underlying cuSolverMp context.""" | ||
| if self._ptr is not None: | ||
| tex.cusolvermp_ctx_destroy(self._ptr) | ||
| try: | ||
| if self._ptr is not None: | ||
| tex.cusolvermp_ctx_destroy(self._ptr) | ||
| finally: | ||
| self._ptr = None | ||
| self._group = None | ||
|
|
||
| def __del__(self) -> None: | ||
| # Called when the context is manually destroyed or during Python teardown | ||
| self.destroy() | ||
|
|
||
|
|
||
| def _get_nccl_comm_ptr(group: dist.ProcessGroup) -> int: | ||
| """Extract the raw NCCL communicator pointer from a PyTorch process group.""" | ||
| """Materialize and borrow a raw NCCL communicator from a process group.""" | ||
| backend = dist.get_backend(group) | ||
| if backend != "nccl": | ||
| raise RuntimeError(f"Newton-Schulz requires NCCL backend, got '{backend}'") | ||
|
|
||
| # ProcessGroupNCCL creates communicators lazily. This device-specific | ||
| # collective ensures that the communicator returned by _comm_ptr() exists | ||
| # and is ready on every group rank before cuSolverMp borrows it. | ||
| dist.barrier(group=group, device_ids=[torch.cuda.current_device()]) | ||
| nccl_backend = group._get_backend(torch.device("cuda")) | ||
| return nccl_backend._comm_ptr() | ||
| comm_ptr = nccl_backend._comm_ptr() | ||
|
Comment on lines
+177
to
+179
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| if not isinstance(comm_ptr, int) or comm_ptr == 0: | ||
| raise RuntimeError("ProcessGroupNCCL returned an invalid communicator pointer") | ||
| return comm_ptr | ||
|
|
||
|
|
||
| def newton_schulz( | ||
|
|
||
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.
__del__may race with CUDA context teardown__del__callsdestroy()which in turn callstex.cusolvermp_ctx_destroy. During interpreter shutdown, CUDA driver state is torn down in an unspecified order relative to Python object finalization. If the CUDA context has already been destroyed when__del__fires,cusolvermp_ctx_destroy(and thecusolverMpDestroyGrid/cusolverMpDestroycalls inside it) may trigger CUDA errors or a segfault. Guarding withtry/except Exception: passinside__del__, or checkingtorch.cuda.is_initialized()before calling into the native layer, would prevent noisy teardown-time crashes in programs that forget to calldestroy()explicitly.