Fix: PY_SSIZE_T_CLEAN macro must be defined for '#' formats #6928
Conversation
AdityaMayukhSom
requested review from
antiagainst,
zhanglx13 and
ptillet
as code owners
AdityaMayukhSom
changed the title
| @@ -1,10 +1,10 @@ | |||
| #define PY_SSIZE_T_CLEAN | |||
There was a problem hiding this comment.
If you only want to fix this file, why trigger other changes?
There was a problem hiding this comment.
I also don't understand
For python 3.12 and older, the macro PY_SSIZE_T_CLEAN must be defined before including Python.h to use all # variants of formats (s#, y#, etc.).
Isn't is already defined before Python.h here?
There was a problem hiding this comment.
Please do clean up your PR
There was a problem hiding this comment.
I unintentionally formatted the python files by mistake, I have removed the unwanted changes.
Regarding your second question, yes it was defined before Python.h, but the error was still present (unsloth version '2025.5.7').
My understanding is that the error was due #include statements written before PY_SSIZE_T_CLEAN was defined, as per the system error message PY_SSIZE_T_CLEAN macro must be defined for '#' forma.
Defining it on top of the file removed the error for me (on python 3.12.10).
There was a problem hiding this comment.
What you did is not consistent with your comment
For python 3.12 and older, the macro PY_SSIZE_T_CLEAN must be defined before including Python.h to use all # variants of formats (s#, y#, etc.).
There was a problem hiding this comment.
This bug seems to still exist. The macro must also be defined in the embedded C source code defined in driver.py, in the make_lanucher function, line 229:
` params.append("&global_scratch")
src = f"""
#include "cuda.h"
#include <stdbool.h>
#include <Python.h>
#include <dlfcn.h>
`
PY_SSIZE_T_CLEAN is not defined before Python.h is included.
|
@AdityaMayukhSom thanks for the fix. I was facing the same issue while training using SFT Trainer. Shifting |
| @@ -227,6 +227,7 @@ def format_of(ty): | |||
| params = [f"&arg{i}" for i, ty in signature.items() if ty != "constexpr"] | |||
| params.append("&global_scratch") | |||
| src = f""" | |||
| #define PY_SSIZE_T_CLEAN | |||
| #include \"cuda.h\" | |||
| #include <stdbool.h> | |||
| #include <Python.h> | |||
There was a problem hiding this comment.
This seems like the only change that is likely to make any difference. All the other cases already defined the macro before including Python.h.
|
(Came from Unsloth unslothai/unsloth#2760, met this issue for Unsloth notebook on RTX 5090. Hopefully this can fix soon, thank you! |
New contributor declaration
I am not making a trivial change, such as fixing a typo in a comment.
I have written a PR description following these rules.
Fixes #5919.
For python 3.12 and older, the macro PY_SSIZE_T_CLEAN must be defined before including Python.h to use all # variants of formats (s#, y#, etc.).
This PR defines PY_SSIZE_T_CLEAN in the first line for both AMD and NVIDIA backend C and python modules. This fixes
SystemError: PY_SSIZE_T_CLEAN macro must be defined for '#' formatson python 3.12 and below whendriver.cfile is dynamically loaded and converted into module.This change is necessary because as of the date of this PR, fine tuning models with unsloth and vLLM only works with python 3.12 and below.
I have run
pre-commit run --from-ref origin/main --to-ref HEAD.Select one of the following.
/testforlittests/unittestfor C++ tests/python/testfor end-to-end testsSelect one of the following.
littests.littests I have added follow these best practices,including the "tests should be minimal" section. (Usually running Python code
and using the instructions it generates is not minimal.)