Skip to content

Fix gpt temporary patch for grpo to happen after compile #4180

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.

Sign up for GitHub

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

Merged
danielhanchen merged 2 commits into from
Mar 8, 2026
Merged

Fix gpt temporary patch for grpo to happen after compile #4180

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94dd4fb
Fix gpt temporary patch for grpo to happen after compile
mmathew23 Mar 8, 2026
1a93b57
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 8, 2026
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
23 changes: 17 additions & 6 deletions unsloth/models/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,21 @@ def prefer_flex_attn_if_supported(model_class, config):
return None


for temporary_patch in TEMPORARY_PATCHES:
temporary_patch()
def _run_temporary_patches(phase):
import inspect

for temporary_patch in TEMPORARY_PATCHES:
try:
sig = inspect.signature(temporary_patch)
if "phase" in sig.parameters:
temporary_patch(phase = phase)
else:
temporary_patch()
except (ValueError, TypeError):
Comment on lines +266 to +267
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Mar 8, 2026

Choose a reason for hiding this comment

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

medium

The except (ValueError, TypeError) block is overly broad and can mask TypeError exceptions that signal a genuine issue with a patch function, such as incorrect arguments. By catching TypeError and then unconditionally calling temporary_patch(), you might suppress the original error and cause a new, potentially more confusing one. It's better to only catch ValueError, which inspect.signature raises for callables it can't inspect (like some C built-ins). A TypeError should generally be allowed to propagate to indicate a problem with the patch or its invocation, leading to faster bug detection.

Suggested change
temporary_patch()
except (ValueError, TypeError):
except ValueError:
temporary_patch()

temporary_patch()
Comment on lines +267 to +268
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 8, 2026

Choose a reason for hiding this comment

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

P2 Badge Narrow TypeError fallback to signature introspection

Catching TypeError around the whole patch invocation can mis-handle real runtime failures inside a temporary patch: if temporary_patch(phase=phase) raises a TypeError from its own logic (not from argument binding), this code retries temporary_patch() and can execute patch side effects twice while obscuring the original failure path. This regression only appears when a patch throws TypeError, but in that case it can leave monkey-patched state inconsistent and make debugging much harder.

Useful? React with 👍 / 👎.



_run_temporary_patches("init")

# =============================================
# Disable some warnings which can get annoying
Expand Down Expand Up @@ -2095,8 +2108,7 @@ def unsloth_compile_transformers(

# Run patches BEFORE compiler so class replacements (e.g. GptOssTopKRouter,
# GptOssExperts) are in place before the compiler caches references to them.
for temporary_patch in TEMPORARY_PATCHES:
temporary_patch()
_run_temporary_patches("pre_compile")

for model_type in model_types:
_unsloth_compile_transformers(
Expand Down Expand Up @@ -2128,8 +2140,7 @@ def unsloth_compile_transformers(
supports_sdpa = supports_sdpa,
)
# Redo patches which override compiler
for temporary_patch in TEMPORARY_PATCHES:
temporary_patch()
_run_temporary_patches("post_compile")
return model_types, supports_sdpa[0]


Expand Down