Repository navigation
Conversation
Signed-off-by: aoshen02 <aoshen@inferact.ai>
There was a problem hiding this comment.
Code Review
This pull request consolidates asynchronous training by removing train_async.py and integrating fully-asynchronous rollout capabilities directly into train.py. It also introduces support for the SUPA accelerator (Biren GPUs), implements disk-spilling and prefetching for routed experts (R3) to optimize host memory usage, and adds a utility to reset the CUDA stack size after model offload. The review feedback highlights several critical robustness improvements, including safely accessing torch.version attributes to prevent AttributeError on standard PyTorch installations, defensively handling potentially None values for args.num_experts to avoid TypeError, and correcting test mocks to avoid attribute errors during unit testing.
| def reset_cuda_stack_size() -> None: | ||
| """Release an enlarged CUDA per-thread stack after model offload.""" | ||
| if torch.version.cuda is None or torch.version.hip is not None or not torch.cuda.is_initialized(): | ||
| return | ||
| torch.cuda.synchronize() | ||
| driver = _cuda_stack_api() | ||
| previous = ctypes.c_size_t() | ||
| error = driver.cuCtxGetLimit(ctypes.byref(previous), 0) # CU_LIMIT_STACK_SIZE | ||
| if error: | ||
| raise RuntimeError(f"cuCtxGetLimit(CU_LIMIT_STACK_SIZE) failed: CUDA error {error}") | ||
| if previous.value <= 1024: | ||
| return | ||
| error = driver.cuCtxSetLimit(0, 1024) | ||
| if error: | ||
| raise RuntimeError(f"cuCtxSetLimit(CU_LIMIT_STACK_SIZE) failed: CUDA error {error}") | ||
| logger.info("Reset CUDA stack limit after offload: %d -> 1024 bytes", previous.value) |
There was a problem hiding this comment.
Accessing torch.version.hip directly will raise an AttributeError on standard (non-ROCm) PyTorch installations because the hip attribute is not defined on the torch.version module in standard builds. To prevent this, use getattr(torch.version, "hip", None) and getattr(torch.version, "cuda", None) to safely check the versions. Additionally, wrapping the entire body of reset_cuda_stack_size in a try...except block is highly recommended to ensure that any unexpected CUDA stack API loading or driver failures do not crash the training process.
def reset_cuda_stack_size() -> None:
"""Release an enlarged CUDA per-thread stack after model offload."""
cuda_version = getattr(torch.version, "cuda", None)
hip_version = getattr(torch.version, "hip", None)
if cuda_version is None or hip_version is not None or not torch.cuda.is_initialized():
return
try:
torch.cuda.synchronize()
driver = _cuda_stack_api()
previous = ctypes.c_size_t()
error = driver.cuCtxGetLimit(ctypes.byref(previous), 0) # CU_LIMIT_STACK_SIZE
if error:
raise RuntimeError(f"cuCtxGetLimit(CU_LIMIT_STACK_SIZE) failed: CUDA error {error}")
if previous.value <= 1024:
return
error = driver.cuCtxSetLimit(0, 1024)
if error:
raise RuntimeError(f"cuCtxSetLimit(CU_LIMIT_STACK_SIZE) failed: CUDA error {error}")
logger.info("Reset CUDA stack limit after offload: %d -> 1024 bytes", previous.value)
except Exception as exc:
logger.warning("Failed to reset CUDA stack size: %s", exc)| monkeypatch.setattr(memory_utils.torch.version, "cuda", cuda) | ||
| monkeypatch.setattr(memory_utils.torch.version, "hip", hip) |
There was a problem hiding this comment.
Using monkeypatch.setattr to set cuda and hip directly on memory_utils.torch.version will raise an AttributeError on standard PyTorch installations because the hip attribute does not exist on the torch.version module. To fix this, mock the entire version attribute as a SimpleNamespace containing both cuda and hip attributes, which is consistent with the other test in this file.
| monkeypatch.setattr(memory_utils.torch.version, "cuda", cuda) | |
| monkeypatch.setattr(memory_utils.torch.version, "hip", hip) | |
| monkeypatch.setattr(memory_utils.torch, "version", SimpleNamespace(cuda=cuda, hip=hip)) |
| if moe_layers: | ||
| # Cast first so uint8 expert ids compare correctly with num_experts=256. | ||
| moe_routes = experts[:, moe_layers, :].to(torch.int64) | ||
| num_experts = int(getattr(args, "num_experts", torch.iinfo(torch.int32).max)) |
There was a problem hiding this comment.
If args.num_experts is explicitly set to None, calling int(None) will raise a TypeError. It is safer to retrieve the value defensively and default to torch.iinfo(torch.int32).max if it is None or not present.
| num_experts = int(getattr(args, "num_experts", torch.iinfo(torch.int32).max)) | |
| num_experts_val = getattr(args, "num_experts", None) | |
| num_experts = int(num_experts_val) if num_experts_val is not None else torch.iinfo(torch.int32).max |
| if sample.rollout_routed_experts is None: | ||
| if sample.loss_mask is None or any(sample.loss_mask): | ||
| return sample | ||
| dtype = torch.uint8 if args.num_experts <= 256 else torch.int32 |
There was a problem hiding this comment.
If args.num_experts is None or not defined on args, accessing args.num_experts directly will raise a TypeError or AttributeError. It is safer to use getattr defensively to check if num_experts is set and is less than or equal to 256.
| dtype = torch.uint8 if args.num_experts <= 256 else torch.int32 | |
| num_experts = getattr(args, "num_experts", None) | |
| dtype = torch.uint8 if num_experts is not None and num_experts <= 256 else torch.int32 |
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
63502a1 to
3f5f82d
Compare
a712b9e to
b600940
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
b600940 to
cd3992f
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
17390d4 to
72bdd14
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
3739f33 to
30ce1fe
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
2e2a7f4 to
927a686
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
|
Exact-head full CI is green: Buildkite #1385 passed all automatic CPU checks and 40/40 GPU jobs on commit |
|
ARM64 follow-up: |
Port #2437 flush scheduling and #2434 reference log-prob replay fix. Replace request-bound TITO weight version with per-output propagation through vLLM #53199, and preserve exact top-p score-centering support. Signed-off-by: aoshen02 <aoshen@inferact.ai>
a3e63cf to
34b247c
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
a476dd9 to
d978296
Compare
…bilities Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
48d31ee to
c5100bd
Compare
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Signed-off-by: aoshen02 <aoshen@inferact.ai>
Summary
Mirror Slime from
4c193f1fthrough8c17b676: 14 applicable merged PRs;Ascend #2424 remains explicitly excluded. Biren/SUPA is mirrored as requested.
Includes score centering, Straw-backed distributed fully-async rollout and
checkpoint continuation, PipelineRL flush, and reference full-vocabulary top-p
handling. Existing Vime-native overlays are retained separately from translations.
Engine patches
The vLLM source pin remains
e9757321527ca1ecd514c07c1418dd2c53da3d19.vllm-pull_weights.patchvllm.patchvllm-score-centering.patchvllm-pd-request-metrics.patchvllm-inflight-queue-diagnostics.patchvllm-aux-output-reset.patchFinal audit
processed_logprobsserver configuration, including external engines, rather than a nonexistent environment variable.Validation
f32a36f1d8f811260f92c1901e808c35ad13c4ea.aosheninferact/vime@sha256:d2d60d7ac5a4fc27299bee5a6ad9f24ba30508f52590dd7f10d4b66516f56cf1.vllm/vime:latestremains unchanged; ARM publication follows full AMD64 CI.