Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/axolotl/integrations/protrain/api/model_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,20 @@
# Reserve 2 GiB for CUDA context + allocator overhead.
_DEFAULT_HEADROOM_BYTES = 2 * (1 << 30)

# A vision tower the text-only profiling batch never exercises is invisible to
# estimate_peak, so reserve extra capacity to force offload instead of OOM.
_VL_VISION_HEADROOM_BYTES = 4 * (1 << 30)


def _has_unprofiled_vision_tower(model: "nn.Module") -> bool:
cfg = getattr(model, "config", None)
if cfg is not None and getattr(cfg, "vision_config", None) is not None:
return True
for name, _ in model.named_modules():
if name.rsplit(".", 1)[-1] in {"visual", "vision_tower", "vision_model"}:
return True
Comment on lines +77 to +84

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Apply ruff format for this file before merge.

CI is currently failing on this file due formatting drift; please commit formatter output.

🧰 Tools
🪛 GitHub Actions: lint / 0_pre-commit.txt

[error] 83-83: ruff-format reformatting changed this file (2 files reformatted). Apply formatting changes committed by the hook.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/axolotl/integrations/protrain/api/model_wrapper.py` around lines 77 - 83,
The file fails CI due to formatting drift; run the ruff formatter on the file
and commit the changes. Specifically, run `ruff format` (or your project's ruff
command) against src/axolotl/integrations/protrain/api/model_wrapper.py and
ensure the function _has_unprofiled_vision_tower and surrounding code are
reformatted, then add and commit the reformatted file so CI passes.

return False

# Slack for allocator frag, framework working set, dataloader workers.
_DEFAULT_CPU_HEADROOM_BYTES = 2 * (1 << 30)

Expand Down Expand Up @@ -2204,8 +2218,17 @@ def protrain_model_wrapper(
# Now derive capacity_bytes / cpu_capacity_bytes from the broadcast-synchronized
# hardware_profile so every rank computes the identical searcher capacity-cutoff.
if _capacity_bytes_caller is None:
_headroom = _DEFAULT_HEADROOM_BYTES
if _has_unprofiled_vision_tower(model):
_headroom += _VL_VISION_HEADROOM_BYTES
LOG.warning(
"ProTrain: vision tower not exercised by the text-only profiling "
"batch; reserving an extra %.1f GiB capacity headroom so the "
"searcher offloads instead of OOMing in the vision forward.",
_VL_VISION_HEADROOM_BYTES / (1 << 30),
)
capacity_bytes = max(
0, int(hardware_profile.gpu_memory_bytes) - _DEFAULT_HEADROOM_BYTES
0, int(hardware_profile.gpu_memory_bytes) - _headroom
)
else:
capacity_bytes = _capacity_bytes_caller
Expand Down
40 changes: 34 additions & 6 deletions src/axolotl/integrations/protrain/args.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,26 +186,43 @@ class ProTrainArgs(BaseModel):
ge=0,
json_schema_extra={
"description": (
"Debug override: force the number of persistent chunks. "
"Bypasses the exhaustive searcher when set alongside the other "
"three overrides."
"Debug override: force the number of persistent chunks. The "
"searcher is bypassed only when all four of "
"n_persist/n_buffer/n_swap/n_checkpoint are set; a partial set "
"is auto-completed with defaults (n_persist=0, n_buffer=2, "
"n_swap=0, n_checkpoint=0) and logs a warning."
)
},
)
protrain_n_buffer_override: int | None = Field(
default=None,
ge=0,
json_schema_extra={"description": "Debug override for n_buffer."},
json_schema_extra={
"description": (
"Debug override for n_buffer. Effective only as part of the full "
"4-knob override set (see protrain_n_persist_override)."
)
},
)
protrain_n_swap_override: int | None = Field(
default=None,
ge=0,
json_schema_extra={"description": "Debug override for n_swap."},
json_schema_extra={
"description": (
"Debug override for n_swap. Effective only as part of the full "
"4-knob override set (see protrain_n_persist_override)."
)
},
)
protrain_n_checkpoint_override: int | None = Field(
default=None,
ge=0,
json_schema_extra={"description": "Debug override for n_checkpoint."},
json_schema_extra={
"description": (
"Debug override for n_checkpoint. Effective only as part of the "
"full 4-knob override set (see protrain_n_persist_override)."
)
},
)
protrain_n_offload_override: int | None = Field(
default=None,
Expand Down Expand Up @@ -506,6 +523,17 @@ class ProTrainArgs(BaseModel):
)
},
)
protrain_lora_mlp_forbid_offload: bool = Field(
default=False,
json_schema_extra={
"description": (
"Re-enable the conservative pin that refuses n_offload>0 "
"candidates when lora_mlp_kernel is on. Off by default since "
"shape-preserving placeholders let the fused MLP kernel compose "
"with offload."
)
},
)

# ------------------------------------------------------------------
# Validators
Expand Down
43 changes: 38 additions & 5 deletions src/axolotl/integrations/protrain/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1642,6 +1642,36 @@ def post_model_load(self, cfg, model: "nn.Module") -> None:
n_offload_override = getattr(cfg, "protrain_n_offload_override", None)
zero3_shard = getattr(cfg, "protrain_zero3_shard", None)

# The wrapper bypasses the searcher only when all four of
# n_persist/n_buffer/n_swap/n_checkpoint are set; a partial set would
# otherwise silently fall back to the auto search. Complete it with safe
# defaults so a partial override does what the user intended.
_core_overrides = {
"protrain_n_persist_override": n_persist_override,
"protrain_n_buffer_override": n_buffer_override,
"protrain_n_swap_override": n_swap_override,
"protrain_n_checkpoint_override": n_checkpoint_override,
}
_set = [k for k, v in _core_overrides.items() if v is not None]
if 0 < len(_set) < 4:
if n_persist_override is None:
n_persist_override = 0
if n_buffer_override is None:
n_buffer_override = 2
if n_swap_override is None:
n_swap_override = 0
if n_checkpoint_override is None:
n_checkpoint_override = 0
LOG.warning(
"ProTrain: partial residency override (set: %s). The override "
"path needs all four of n_persist/n_buffer/n_swap/n_checkpoint "
"or it silently falls back to the auto search. Filled unset "
"knobs: n_persist=%d n_buffer=%d n_swap=%d n_checkpoint=%d. Set "
"all four explicitly to suppress this.",
", ".join(_set), n_persist_override, n_buffer_override,
n_swap_override, n_checkpoint_override,
)
Comment on lines +1665 to +1676

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Apply ruff format for this file before merge.

CI indicates formatting changes are still required in this file.

🧰 Tools
🪛 GitHub Actions: lint / 0_pre-commit.txt

[error] 1668-1668: ruff-format reformatting changed this file (2 files reformatted). Apply formatting changes committed by the hook.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/axolotl/integrations/protrain/plugin.py` around lines 1665 - 1673, The
file fails the linter/formatter; run the ruff formatter on
src/axolotl/integrations/protrain/plugin.py (or apply ruff format to the working
tree) to fix formatting issues around the LOG.warning call and surrounding code
(e.g., the LOG.warning invocation that references _set, n_persist_override,
n_buffer_override, n_swap_override, n_checkpoint_override). Ensure the formatted
output conforms to project ruff settings and then re-run CI.


# auto_mode default True; wrapper picks (force_persist, zero3) post-search.
auto_mode = getattr(cfg, "protrain_auto_mode", True)
if auto_mode is None:
Expand All @@ -1664,13 +1694,16 @@ def post_model_load(self, cfg, model: "nn.Module") -> None:
"(force_all_persistent=False, zero3_shard=False)."
)

# Fused LoRA MLP backward kernel + offloaded-activation chunk placeholders
# crash with LoRA_MLPBackward shape-mismatch (v61); pin n_offload=0.
forbid_activation_offload = bool(getattr(cfg, "lora_mlp_kernel", False))
# The v61 LoRA_MLPBackward shape-mismatch is fixed by the unconditional
# shape-preserving placeholders, so the fused MLP kernel now composes
# with offload; opt back into the old conservative pin if needed.
forbid_activation_offload = bool(
getattr(cfg, "protrain_lora_mlp_forbid_offload", False)
)
if forbid_activation_offload:
LOG.info(
"ProTrain: cfg.lora_mlp_kernel=True; searcher will refuse "
"n_offload>0 candidates."
"ProTrain: protrain_lora_mlp_forbid_offload=True; searcher will "
"refuse n_offload>0 candidates."
)
Comment on lines +1703 to 1710

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the wrapper docstring to match this new knob source.

This hunk now uses protrain_lora_mlp_forbid_offload, but protrain_model_wrapper docs still describe forbid_activation_offload as coming from cfg.lora_mlp_kernel.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/axolotl/integrations/protrain/plugin.py` around lines 1700 - 1707, Update
the protrain_model_wrapper docstring to reflect that the
forbid_activation_offload flag is read from cfg.protrain_lora_mlp_forbid_offload
(mapped to local variable forbid_activation_offload) instead of
cfg.lora_mlp_kernel; edit the docstring text and any parameter descriptions to
mention protrain_lora_mlp_forbid_offload and the behavior ("refuse n_offload>0
candidates") so the docs match the code path that sets
forbid_activation_offload.


# PR #17c: defensive searcher tie-break on non-NVLink multi-rank rigs.
Expand Down
Loading