Skip to content

Patch before compile? #3995

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

Closed
Datta0 wants to merge 2 commits into from
Closed

Patch before compile? #3995

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fc4ad33
Patch before compile?
Datta0 Feb 6, 2026
b5d4e86
Merge branch 'main' into fix_gpt_oss
Datta0 Feb 9, 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
6 changes: 6 additions & 0 deletions unsloth/models/_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1966,6 +1966,12 @@ def unsloth_compile_transformers(
return model_types, False

supports_sdpa = [True]

# 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()
Comment on lines +1972 to +1973
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Feb 7, 2026

Choose a reason for hiding this comment

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

medium

This loop applies temporary patches, which is also done at the module level (lines 217-218). While the comment explains the need for patching right before compilation, this introduces a second application of the patches before the compilation loop. If these patches are idempotent, this is safe, but it could be slightly inefficient. For long-term maintainability, it would be ideal to have a single, clear point of application or a mechanism to prevent redundant patching. Since this change seems to be a targeted fix, it's acceptable, but it might be worth considering a small refactor to consolidate the patching logic in the future to avoid confusion.


for model_type in model_types:
_unsloth_compile_transformers(
model_type,
Expand Down