[GH-ISSUE #46] Pre-RFC: MiniMax H3 Ulysses sequence-parallel integration boundary #1

New issue

Open

opened 2026-10-26 22:01:11 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:01:11 +00:00

Owner

Originally created by @RagnarokChan on GitHub (Sep 21, 2026).
Original GitHub issue: https://github.com/Comfy-Org/rfcs/issues/46

Purpose

Following the pre-RFC feedback process, I would like feedback on a supported integration path for MiniMax H3 Ulysses sequence parallelism, before proposing changes to ComfyUI core.

The concrete goal is to reduce dependence on copied H3 internals and private ModelPatcher state in a working custom-node implementation. This is not an implementation PR or a request to adopt a new general MultiGPU framework.

Existing implementation and attribution

The original implementation is buqi-code/buqi-minimax-h3-multigpu, published by buqi-code under MIT. The Ulysses implementation is the author's work, not a new algorithm from this compatibility effort. Relevant ComfyUI code and notices must also be preserved; any eventual core contribution must meet the destination project's licensing and contribution requirements.

A compatibility PR to the author keeps the original architecture:

Rank 0 is ComfyUI; additional ranks are separate worker processes holding DiT replicas.
NCCL transports tensors; a Gloo subgroup transports control metadata.
The author's sequence-sharded forward and all-to-all path are retained.
The actual downstream ModelPatcher's static LoRA patches are synchronized before denoising, with payload hash, key/version and rank acknowledgment checks.
Text encoding and normal VAE decoding stay on rank 0. Sequence parallelism does not pool the GPUs' weight memory.

The custom node remains independently usable while any core proposal is discussed.

Evidence, with limits

The fixed source and change record and sanitized validation records describe tests on Linux, two RTX 2080 Ti GPUs reporting 22 GiB each, NVLink NV2, PyTorch 2.14.0+cu130, and ComfyUI 0.36.0 at 99073836d45f66053c45ba8564984e6def9cebba.

Recorded runs include native single-GPU bypass, two-GPU INT8 ConvRot H3, 4-step and 8-step Turbo LoRA (208 patch keys per rank), clearing back to Base (0 keys), I2V smoke, and controlled cancellation / idle-worker recreation. The record distinguishes intermediate failures from later successful runs. The fresh 4/8-step 832×480, 124-frame runs and later lifecycle regressions are identified separately by source revision.

The earlier warmed Base baseline was 935.28 s single GPU versus 534.30 s dual GPU (about 1.75× end-to-end). This is historical evidence of viability, not a performance claim for current ComfyUI HEAD, and runs with native numerical verification enabled are not fair timing benchmarks.

Full FP16 H3, FP8/NVFP4 H3, full reference workflows, SP VAE, arbitrary hooks and more than two ranks are not validated by this work. FP16 VAE/storage does not mean FP16 DiT computation: the tested native H3 path uses FP32 compute on SM75. Controlled recovery does not establish fault tolerance for arbitrary failures inside collectives. The existing same-host patch IPC is a trusted transport, not a security boundary or proposed public serialization standard.

Integration questions

H3 boundary: Would maintainers prefer a narrowly supported H3 extension interface with SP remaining a custom node, or consider an H3-specific core implementation after an RFC? Shared layout/conditioning/modulation helpers could reduce forward drift. An attention-only hook cannot by itself preserve sequence sharding through the MLP and the rest of the forward.
Worker model and patch state: What should a supported cross-process worker consume: a model loader description and patch application description, a patch-only snapshot/version interface, or another existing contract? The current node uses an ON_PRE_RUN callback on the actual downstream patcher, but its worker still depends on private patch/version fields and repeats native DynamicVRAM bootstrap. Those are integration costs, not APIs being proposed for adoption as-is.
Lifecycle ownership: If cross-process SP is an acceptable direction, where should worker creation, device ownership, cancellation, teardown and patch-readiness checks live? The MultiGPUThreadPool and clone setup inspected at current HEAD are same-process facilities, not drop-in owners of the existing process-based NCCL ranks. This code inspection is separate from the older, pinned GPU test environment.

I found the broader Ray/parallelism discussion #6647, including work on sequence parallelism. This proposal is narrower: one model's supported integration boundary, with no requirement to add Ray, FSDP or a scheduler redesign.

My preference is to agree on the smallest useful H3-facing interface first, then prepare an RFC or focused PR as maintainers recommend. H3 packed layout, modality masks and Ulysses integration should remain model-specific; shared lifecycle primitives should only be proposed when their reuse is justified. There is no assumption that the current custom node should be copied wholesale into core.

Would this be a useful direction for a contribution, and which of the boundaries above should be addressed first? If an existing effort already owns this interface, I would be happy to align with it.

Originally created by @RagnarokChan on GitHub (Sep 21, 2026). Original GitHub issue: https://github.com/Comfy-Org/rfcs/issues/46 ## Purpose Following the [pre-RFC feedback process](https://github.com/Comfy-Org/rfcs#gathering-feedback-before-submitting), I would like feedback on a supported integration path for **MiniMax H3 Ulysses sequence parallelism**, before proposing changes to ComfyUI core. The concrete goal is to reduce dependence on copied H3 internals and private ModelPatcher state in a working custom-node implementation. This is not an implementation PR or a request to adopt a new general MultiGPU framework. ## Existing implementation and attribution The original implementation is [buqi-code/buqi-minimax-h3-multigpu](https://github.com/buqi-code/buqi-minimax-h3-multigpu), published by buqi-code under MIT. The Ulysses implementation is the author's work, not a new algorithm from this compatibility effort. Relevant ComfyUI code and notices must also be preserved; any eventual core contribution must meet the destination project's licensing and contribution requirements. A [compatibility PR to the author](https://github.com/buqi-code/buqi-minimax-h3-multigpu/pull/6) keeps the original architecture: - Rank 0 is ComfyUI; additional ranks are separate worker processes holding DiT replicas. - NCCL transports tensors; a Gloo subgroup transports control metadata. - The author's sequence-sharded forward and all-to-all path are retained. - The actual downstream ModelPatcher's static LoRA patches are synchronized before denoising, with payload hash, key/version and rank acknowledgment checks. - Text encoding and normal VAE decoding stay on rank 0. Sequence parallelism does **not** pool the GPUs' weight memory. The custom node remains independently usable while any core proposal is discussed. ## Evidence, with limits The [fixed source and change record](https://github.com/RagnarokChan/buqi-minimax-h3-multigpu/blob/d1a9f7feca69c9958a2b224691e12b917835419f/CHANGELOG_LOCAL.md) and [sanitized validation records](https://github.com/RagnarokChan/buqi-minimax-h3-multigpu/blob/d1a9f7feca69c9958a2b224691e12b917835419f/evidence/pr_validation.json) describe tests on Linux, two RTX 2080 Ti GPUs reporting 22 GiB each, NVLink NV2, PyTorch 2.14.0+cu130, and ComfyUI 0.36.0 at `99073836d45f66053c45ba8564984e6def9cebba`. Recorded runs include native single-GPU bypass, two-GPU INT8 ConvRot H3, 4-step and 8-step Turbo LoRA (208 patch keys per rank), clearing back to Base (0 keys), I2V smoke, and controlled cancellation / idle-worker recreation. The record distinguishes intermediate failures from later successful runs. The fresh 4/8-step 832×480, 124-frame runs and later lifecycle regressions are identified separately by source revision. The earlier warmed Base baseline was 935.28 s single GPU versus 534.30 s dual GPU (about 1.75× end-to-end). This is historical evidence of viability, **not a performance claim for current ComfyUI HEAD**, and runs with native numerical verification enabled are not fair timing benchmarks. Full FP16 H3, FP8/NVFP4 H3, full reference workflows, SP VAE, arbitrary hooks and more than two ranks are not validated by this work. FP16 VAE/storage does not mean FP16 DiT computation: the tested native H3 path uses FP32 compute on SM75. Controlled recovery does not establish fault tolerance for arbitrary failures inside collectives. The existing same-host patch IPC is a trusted transport, not a security boundary or proposed public serialization standard. ## Integration questions 1. **H3 boundary:** Would maintainers prefer a narrowly supported H3 extension interface with SP remaining a custom node, or consider an H3-specific core implementation after an RFC? Shared layout/conditioning/modulation helpers could reduce forward drift. An attention-only hook cannot by itself preserve sequence sharding through the MLP and the rest of the forward. 2. **Worker model and patch state:** What should a supported cross-process worker consume: a model loader description and patch application description, a patch-only snapshot/version interface, or another existing contract? The current node uses an ON_PRE_RUN callback on the actual downstream patcher, but its worker still depends on private patch/version fields and repeats native DynamicVRAM bootstrap. Those are integration costs, not APIs being proposed for adoption as-is. 3. **Lifecycle ownership:** If cross-process SP is an acceptable direction, where should worker creation, device ownership, cancellation, teardown and patch-readiness checks live? The [MultiGPUThreadPool and clone setup inspected at current HEAD](https://github.com/Comfy-Org/ComfyUI/blob/c194dd00cd42aa18d9dbf27d977bf6b85d9ea565/comfy/multigpu.py) are same-process facilities, not drop-in owners of the existing process-based NCCL ranks. This code inspection is separate from the older, pinned GPU test environment. ## Proposed next step and related work I found the broader [Ray/parallelism discussion #6647](https://github.com/Comfy-Org/ComfyUI/discussions/6647), including work on sequence parallelism. This proposal is narrower: one model's supported integration boundary, with no requirement to add Ray, FSDP or a scheduler redesign. My preference is to agree on the smallest useful H3-facing interface first, then prepare an RFC or focused PR as maintainers recommend. H3 packed layout, modality masks and Ulysses integration should remain model-specific; shared lifecycle primitives should only be proposed when their reuse is justified. There is no assumption that the current custom node should be copied wholesale into core. Would this be a useful direction for a contribution, and which of the boundaries above should be addressed first? If an existing effort already owns this interface, I would be happy to align with it.

bsakaguchi referenced this issue

2026-10-26 22:01:16 +00:00

[PR #1] [MERGED] RFC: object_info (v2) #3