Skip to content

Commit dd925a9

Browse files
committed
Two more things only macOS and Windows could tell us
Ran the branch on macos-14 and windows-latest, which no workflow in this repo does. Two failures, both in the tests rather than in saving_utils. test_a_local_4bit_copy_still_merges_when_the_config_fetch_is_offline uses the same `Outputs/MyModel` over an on-disk `outputs/mymodel` shape as its sibling file, and for the same reason: that case difference is the only way to make os.path.exists(name) miss while check_local_model_exists(name) hits. On APFS and NTFS os.path.exists answers True, the local branch is taken, and the Hub round trip never happens. macOS failed on the path comparison and Windows on `resolved[1] is True`, which is the same cause wearing two symptoms. Probe and skip with the reason, as the sibling now does. test_a_local_base_still_merges_with_the_hub_down died in MPS: RuntimeError: MPS backend out of memory (MPS allocated: 8.00 MiB, ... max allowed: 7.93 GiB). Tried to allocate 256 bytes on shared pool. Refusing 256 bytes with 7.93 GiB allowed is the macos-14 runner's virtualized MPS, not this branch, and _active_merge_device prefers MPS on purpose. But it exposed a claim in that file that was not true: the docstring said CPU-only while set_offline_cpu_env only sets UNSLOTH_ALLOW_CPU, which permits CPU rather than requiring it. On this box the merge math was running on cuda. These cells assert control flow, whether the merge raises and whether it writes, so the device is incidental and letting the host pick it makes the result depend on hardware the assertions have no opinion about. Pinned to CPU and said so. Verified the pin is real rather than decorative: _active_merge_device() answers cuda here unpatched, and the merged tensors come back on cpu with the fixture. Behavioral gate 564 passed, 2 skipped.
1 parent cb49561 commit dd925a9

2 files changed

Lines changed: 45 additions & 4 deletions

File tree

tests/test_gated_and_offline_config_classification.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,28 @@
7979
_ON_DISK = ("outputs", "mymodel")
8080

8181

82+
def _require_a_case_sensitive_filesystem(tmp_path):
83+
"""Same precondition, same reason as in that sibling file, stated not assumed.
84+
85+
A case difference is the only thing that makes `os.path.exists(name)` miss
86+
while `check_local_model_exists(name)` hits, so the shape cannot be written
87+
filesystem independently. On macOS APFS and Windows NTFS `os.path.exists`
88+
answers True for `Outputs/MyModel`, the local branch is taken, and the
89+
resolved path comes back in the requested casing instead of the on-disk one.
90+
91+
macos-14 and windows-latest both fail here without this: macOS on the path
92+
comparison, Windows on `resolved[1] is True`.
93+
"""
94+
probe = tmp_path / "case_probe"
95+
probe.mkdir(exist_ok = True)
96+
if os.path.exists(str(tmp_path / "CASE_PROBE")):
97+
pytest.skip(
98+
"case insensitive filesystem: os.path.exists would resolve "
99+
f"{_REQUESTED!r} locally and skip the Hub branch under test"
100+
)
101+
pass
102+
103+
82104
class _StubResponse:
83105
"""The attributes `HfHubHTTPError.__init__` actually reads on 1.x."""
84106
status_code = 403
@@ -284,6 +306,7 @@ def test_a_local_4bit_copy_still_merges_when_the_config_fetch_is_offline(
284306
):
285307
"""The new raise must still reach the local fallback, exactly as a 429 does.
286308
Both 4bit merges fold LoRA into weights already in memory."""
309+
_require_a_case_sensitive_filesystem(tmp_path)
287310
directory = _make_local_model(tmp_path.joinpath(*_ON_DISK), quant_config = _NF4)
288311
monkeypatch.chdir(tmp_path)
289312
_patch_ls_present(monkeypatch)

tests/test_merge_e2e_hub_unreachable.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,18 @@
3030
things a user experiences: an exception is raised, and no output directory is left
3131
behind that could be mistaken for a successful export.
3232
33-
Uses the tiny synthetic models from `_merge_e2e_helpers`, so this is CPU-only and
34-
sub-second. `set_offline_cpu_env()` there already forbids the network; the Hub
35-
entry points are monkeypatched on top so a failure is deterministic rather than
36-
dependent on being genuinely offline.
33+
Uses the tiny synthetic models from `_merge_e2e_helpers`, so this is sub-second.
34+
`set_offline_cpu_env()` there already forbids the network; the Hub entry points are
35+
monkeypatched on top so a failure is deterministic rather than dependent on being
36+
genuinely offline.
37+
38+
The merge math is pinned to CPU. `set_offline_cpu_env()` sets `UNSLOTH_ALLOW_CPU`,
39+
which permits CPU rather than requiring it, so `_active_merge_device()` still
40+
prefers whatever accelerator is present: CUDA on a training box, MPS on a Mac. What
41+
is under test here is control flow, whether the merge raises and whether it writes,
42+
so the device is incidental, and leaving it to the host makes the result depend on
43+
hardware these assertions have no opinion about. macos-14 showed why: its
44+
virtualized MPS reports 7.93 GiB available and then refuses a 256 byte allocation.
3745
"""
3846

3947
from __future__ import annotations
@@ -50,6 +58,16 @@
5058
FAMILY = "llama"
5159

5260

61+
@pytest.fixture(autouse = True)
62+
def _merge_on_the_cpu(monkeypatch):
63+
"""Pin `_merge_lora`'s device, for the reason in the module docstring.
64+
65+
`_active_merge_device` is `lru_cache`d, so replacing the module attribute is
66+
both what takes effect and what gets undone cleanly by monkeypatch.
67+
"""
68+
monkeypatch.setattr(saving_utils, "_active_merge_device", lambda: "cpu")
69+
70+
5371
def _skip_if_missing():
5472
if not H.family_available(FAMILY):
5573
pytest.skip(f"{FAMILY} unavailable in this transformers")

0 commit comments

Comments
 (0)