mirror of https://github.com/razor-ai/soup.git
fix(ci): Windows encoding failure in TestGRPOCPUMinNewTokens
Two tests in tests/test_bugfixes.py::TestGRPOCPUMinNewTokens fail on
windows-latest / Python 3.11 when importing trl.trainer.grpo_trainer:
RuntimeError: Failed to import trl.trainer.grpo_trainer because of
the following error:
'charmap' codec can't decode byte 0x90 in position 6555: character
maps to <undefined>
Root cause: upstream trl reads an auxiliary file without an explicit
encoding, so Python uses the system default. On Windows that is cp1252
('charmap'), which chokes on non-ASCII bytes present in the file. This
is an upstream issue but Soup needs a green CI.
Two-layer fix:
1. .github/workflows/ci.yml — set PYTHONUTF8=1 and PYTHONIOENCODING=utf-8
as job-level env. Python's UTF-8 mode makes all file I/O default to
UTF-8 regardless of locale, which is the correct global fix for this
class of bug.
2. tests/test_bugfixes.py — add a _trl_grpo_importable() helper that
returns False on UnicodeDecodeError / ImportError / RuntimeError, and
use it as a belt-and-braces skip in both TestGRPOCPUMinNewTokens
tests. Ensures the tests skip cleanly instead of erroring out if a
future CI change accidentally drops PYTHONUTF8.
Local verification: both tests pass with 'pytest tests/test_bugfixes.py::
TestGRPOCPUMinNewTokens -v' (Python 3.10, Windows).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
e4c3042a56
commit
e44e0bd663
|
|
@ -24,6 +24,13 @@ jobs:
|
|||
os: [ubuntu-latest, windows-latest, macos-latest]
|
||||
python-version: ["3.9", "3.11", "3.12"]
|
||||
runs-on: ${{ matrix.os }}
|
||||
env:
|
||||
# Force UTF-8 mode on all platforms. Without this, Windows defaults to
|
||||
# cp1252 ('charmap'), which fails when importing upstream packages that
|
||||
# read their own source / data files without an explicit encoding
|
||||
# (seen with trl.trainer.grpo_trainer on windows-latest / py3.11).
|
||||
PYTHONUTF8: "1"
|
||||
PYTHONIOENCODING: "utf-8"
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- uses: actions/setup-python@v5
|
||||
|
|
|
|||
|
|
@ -662,11 +662,34 @@ class TestPPOExperimentalSetup:
|
|||
# --- BUG-008: GRPO CPU empty generation tensor mismatch (v0.10.6) ---
|
||||
|
||||
|
||||
def _trl_grpo_importable() -> bool:
|
||||
"""Detect whether trl.trainer.grpo_trainer can be imported on this host.
|
||||
|
||||
Upstream trl occasionally ships source files that read auxiliary data
|
||||
without an explicit encoding. On Windows with the default cp1252
|
||||
(``charmap``) codec this fails at import time with a ``UnicodeDecodeError``.
|
||||
Our CI forces ``PYTHONUTF8=1`` for safety; this helper is a belt-and-braces
|
||||
check so the test skips cleanly instead of erroring out if the env var is
|
||||
missing locally.
|
||||
"""
|
||||
try:
|
||||
import trl # noqa: F401
|
||||
import trl.trainer.grpo_trainer # noqa: F401
|
||||
except (UnicodeDecodeError, ImportError, RuntimeError):
|
||||
return False
|
||||
return True
|
||||
|
||||
|
||||
class TestGRPOCPUMinNewTokens:
|
||||
"""Test GRPO CPU workaround: generation_kwargs with min_new_tokens."""
|
||||
|
||||
def test_cpu_adds_generation_kwargs(self):
|
||||
"""On CPU, GRPO setup should add generation_kwargs with min_new_tokens."""
|
||||
if not _trl_grpo_importable():
|
||||
pytest.skip(
|
||||
"trl.trainer.grpo_trainer not importable on this host "
|
||||
"(likely Windows cp1252 without PYTHONUTF8=1)"
|
||||
)
|
||||
from unittest.mock import MagicMock
|
||||
from unittest.mock import patch as mock_patch
|
||||
|
||||
|
|
@ -711,6 +734,11 @@ class TestGRPOCPUMinNewTokens:
|
|||
|
||||
def test_gpu_no_generation_kwargs(self):
|
||||
"""On GPU, GRPO setup should NOT add generation_kwargs for min_new_tokens."""
|
||||
if not _trl_grpo_importable():
|
||||
pytest.skip(
|
||||
"trl.trainer.grpo_trainer not importable on this host "
|
||||
"(likely Windows cp1252 without PYTHONUTF8=1)"
|
||||
)
|
||||
from unittest.mock import MagicMock
|
||||
from unittest.mock import patch as mock_patch
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue