Skip to content

Commit 381a30c

Browse files
shoumikhinr
andauthored
Declare coremltools and scikit-learn only where they work (pytorch#22090)
### Summary Two dependency markers in the wheel do not match where the packages they name are usable. **1. coremltools on Linux aarch64.** coremltools is declared on Darwin and on Linux. Linux is deliberate: the Core ML export flow runs there, so a model can be lowered for Apple hardware from a Linux machine. The problem is that the marker cannot tell the two Linux architectures apart, and coremltools publishes no build for Linux aarch64. On that architecture pip falls back to the source archive and produces a pure-Python install with none of the compiled extensions. Measured with coremltools 9.0 on a Linux aarch64 machine: ``` Tag: py3-none-any Root-Is-Purelib: true $ find <site-packages>/coremltools -name '*.so' # no output at all IMPORT FAIL coremltools.libmilstoragepython ModuleNotFoundError IMPORT FAIL coremltools.libcoremlpython ModuleNotFoundError IMPORT FAIL coremltools.libmodelpackage ModuleNotFoundError ``` It imports, it reports version 9.0, and then it cannot write a model, because `libmilstoragepython` is what stores the weights of an mlprogram. The same version on Linux x86_64 installs `coremltools-9.0-cp3XX-none-manylinux1_x86_64.whl`, which carries `libmilstoragepython.so` and `libmodelpackage.so`. So the Linux half of the marker is narrowed to x86_64, and users on other Linux architectures are no longer handed a package that looks installed and cannot do the job. **2. scikit-learn has no marker at all.** It is there for Core ML palettization, so it should follow coremltools, but instead it installs everywhere, including Windows, where coremltools is already excluded. A Windows install therefore carries scikit-learn and scipy for a backend it cannot use. This gives it the same marker. ### Effect by platform For coremltools, macOS and Linux x86_64 are unchanged at every supported Python version. The Python 3.14 exclusion was already in the marker before this change. What changes is Linux on any architecture other than x86_64. For scikit-learn, it is no longer installed on Windows, on non-x86_64 Linux, or on Python 3.14, and scipy no longer arrives behind it there. Nothing in the shipped library imports either one at module scope. The one library module that uses scipy, the TurboQuant codebook solver in `extension/llm/modules/turboquant/codebook.py`, already imports it inside the function and raises a message telling the user to install it. That module is not example-only: `extension/llm/modules/turboquant/kv_cache.py` imports it and calls it from `TurboQuantKVCache.__init__`, and `backends/mlx/llm/turboquant_cache.py` re-exports that class. See the install-step note below. There is no new extra. There is one new install step, on the platforms where scipy stops arriving: a wheel user on Windows, non-x86_64 Linux, or Python 3.14 who constructs `TurboQuantKVCache` now needs `pip install scipy`, because `TurboQuantKVCache.__init__` calls the codebook solver unconditionally. scipy was never declared in `setup.py`; it only ever arrived as scikit-learn's transitive dependency, so gating scikit-learn gates it too. The guarded import names scipy and the install command, so the failure is actionable rather than a bare `ModuleNotFoundError`, but it is a real change for those users. ### Follow-on changes - `conftest.py` drops `backends/apple/coreml` from collection wherever coremltools is not declared. Every test file there that defines tests imports coremltools at module scope, so collection would otherwise fail. Windows was already covered by an existing rule; this extends it to the other Linux architectures. - A new test, `.ci/scripts/tests/test_coreml_markers.py`. The rule for where coremltools can be installed is now written twice, once as a PEP 508 marker in `setup.py` and once as a Python condition in `conftest.py`, in two different languages, with only a comment holding them together. The test reads both out of their real files with `ast` rather than restating them a third time, and checks that they agree across nine platforms and six Python versions. If one is changed and the other is not, this fails in CI instead of failing at collection with a bare `ModuleNotFoundError`. - scikit-learn and scipy move to `requirements-examples.txt`. Two example scripts import scikit-learn, `examples/openvino/aot_optimize_and_infer.py` and `examples/qualcomm/scripts/mobilebert_fine_tune.py`, and three Qualcomm example scripts import scipy directly. Both used to arrive by accident, scikit-learn as an unmarked base dependency and scipy as its transitive, and neither was ever declared where it is used. `requirements-examples.txt` is where example-only dependencies already live, next to timm and transformers, and it is what `install_requirements.py` and the example CI jobs install. Putting scikit-learn in the `openvino` extra instead would not have been enough, since that script also needs timm and transformers, which the extra does not carry. ### Test plan Evaluated both markers with `packaging` across macOS, Linux x86_64, Linux aarch64, Windows and Python 3.10 through 3.15: ``` coremltools linux x86_64 py3.10 -> True linux aarch64 py3.10 -> False linux x86_64 py3.13 -> True linux x86_64 py3.14 -> False darwin arm64 py3.12 -> True windows AMD64 py3.12 -> False ``` scikit-learn now evaluates identically, and the `conftest.py` condition agrees with the coremltools marker on every one of those combinations. That is what the new test covers, and it passes: ``` 167 passed ``` Confirmed the test can fail. Widening the `conftest.py` condition to also accept Linux aarch64 turns four cases red with a message naming the platform and the Python version: ``` AssertionError: on Linux/aarch64 with Python 3.13 setup.py does not declare coremltools while conftest.py collects the Core ML tests, so pytest will either fail at collection on a missing import or silently skip tests that could have run ``` Confirmed the crippled install by hand on a Linux aarch64 machine, as shown above, and confirmed on Linux x86_64 that the published wheel for the same version ships the compiled extensions. The unit test jobs that collect the Core ML tests are the Linux x86_64 job on Python 3.10 and the macOS arm64 job on Python 3.11. Both are platforms where coremltools is still declared, so those tests collect and run exactly as before. The Windows job already skipped the whole Apple directory. --------- Co-authored-by: r <r@e>
1 parent 1e716a6 commit 381a30c

6 files changed

Lines changed: 374 additions & 4 deletions

File tree

Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,322 @@
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
2+
# All rights reserved.
3+
#
4+
# This source code is licensed under the BSD-style license found in the
5+
# LICENSE file in the root directory of this source tree.
6+
7+
"""Unit tests that the Core ML availability rules agree.
8+
9+
Where coremltools can be installed is written down three times: as a dependency marker in
10+
setup.py, as a condition in conftest.py that drops the Core ML tests from collection, and
11+
as `is_supported_platform_for_coreml_lowering` in export/utils.py, which callers use as an
12+
import guard. They are written in different languages, PEP 508 and Python, so nothing but a
13+
comment keeps them together. When they drift, the wheel declares a package on a platform
14+
the test run assumes is absent, or the other way round, and pytest fails at collection with
15+
a bare ModuleNotFoundError.
16+
17+
The rules are read out of the three files rather than restated here, so a change to any one
18+
of them is exercised by this test instead of being duplicated a fourth time.
19+
"""
20+
21+
import ast
22+
import fnmatch
23+
import re
24+
from pathlib import Path
25+
from types import SimpleNamespace
26+
from typing import NamedTuple
27+
28+
import pytest
29+
from packaging.markers import Marker
30+
31+
REPO_ROOT = Path(__file__).resolve().parents[3]
32+
33+
# platform_system, platform_machine, sys_platform
34+
PLATFORMS = [
35+
("Darwin", "arm64", "darwin"),
36+
("Darwin", "x86_64", "darwin"),
37+
("Linux", "x86_64", "linux"),
38+
("Linux", "aarch64", "linux"),
39+
("Linux", "armv7l", "linux"),
40+
("Linux", "ppc64le", "linux"),
41+
("Linux", "s390x", "linux"),
42+
("Windows", "AMD64", "win32"),
43+
("Windows", "ARM64", "win32"),
44+
]
45+
46+
# Every version in requires-python, plus one past the end.
47+
PYTHONS = [(3, 10), (3, 11), (3, 12), (3, 13), (3, 14), (3, 15)]
48+
49+
50+
class _VersionInfo(NamedTuple):
51+
"""Stands in for sys.version_info: compares as a tuple, has .major/.minor."""
52+
53+
major: int
54+
minor: int
55+
56+
57+
def _normalise(name: str) -> str:
58+
# PEP 503: runs of -, _ and . collapse to a single -, then casefold.
59+
return re.sub(r"[-_.]+", "-", name).lower()
60+
61+
62+
def _project_name(requirement: str) -> str:
63+
# Strip everything a PEP 508 requirement can carry after the project name: an extras
64+
# list, a version specifier, an environment marker, or a URL.
65+
head = re.split(r"[\[<>=!~;@\s]", requirement.strip(), maxsplit=1)[0]
66+
return _normalise(head)
67+
68+
69+
def _base_dependency(name: str) -> str:
70+
"""Return the requirement string for `name` from setup.py's _base_dependencies()."""
71+
# Compare PEP 503 normalised project names rather than using startswith, which would
72+
# match a longer name that merely begins with this one (scikit-learn against
73+
# scikit-learn-intelex) and would also match the function's own docstring, since that
74+
# is just another string constant in the body.
75+
wanted = _normalise(name)
76+
tree = ast.parse((REPO_ROOT / "setup.py").read_text())
77+
for node in ast.walk(tree):
78+
if not (
79+
isinstance(node, ast.FunctionDef) and node.name == "_base_dependencies"
80+
):
81+
continue
82+
for constant in ast.walk(node):
83+
if not (
84+
isinstance(constant, ast.Constant) and isinstance(constant.value, str)
85+
):
86+
continue
87+
if _project_name(constant.value) == wanted:
88+
return constant.value
89+
raise AssertionError(f"no {name} requirement in setup.py _base_dependencies()")
90+
91+
92+
def _conftest_condition() -> str:
93+
"""Return the source of conftest.py's _coremltools_is_declared expression."""
94+
tree = ast.parse((REPO_ROOT / "conftest.py").read_text())
95+
for node in ast.walk(tree):
96+
if isinstance(node, ast.Assign) and any(
97+
isinstance(target, ast.Name) and target.id == "_coremltools_is_declared"
98+
for target in node.targets
99+
):
100+
return ast.unparse(node.value)
101+
raise AssertionError("no _coremltools_is_declared assignment in conftest.py")
102+
103+
104+
def _coreml_ignore_globs() -> list[str]:
105+
"""Return the globs conftest.py adds when _coremltools_is_declared is false.
106+
107+
Reads the `if not _coremltools_is_declared:` statement rather than the module's
108+
resulting state, because importing conftest.py here would evaluate the condition
109+
for the interpreter running this test instead of for the platforms under test.
110+
"""
111+
tree = ast.parse((REPO_ROOT / "conftest.py").read_text())
112+
for node in ast.walk(tree):
113+
if not isinstance(node, ast.If):
114+
continue
115+
test = node.test
116+
# `if not _coremltools_is_declared:`
117+
if not (
118+
isinstance(test, ast.UnaryOp)
119+
and isinstance(test.op, ast.Not)
120+
and isinstance(test.operand, ast.Name)
121+
and test.operand.id == "_coremltools_is_declared"
122+
):
123+
continue
124+
globs: list[str] = []
125+
for stmt in node.body:
126+
# `collect_ignore_glob += [...]` or `collect_ignore_glob = [...]`
127+
targets = (
128+
[stmt.target]
129+
if isinstance(stmt, ast.AugAssign)
130+
else stmt.targets if isinstance(stmt, ast.Assign) else []
131+
)
132+
if not any(
133+
isinstance(t, ast.Name) and t.id == "collect_ignore_glob"
134+
for t in targets
135+
):
136+
continue
137+
for element in ast.walk(stmt.value):
138+
if isinstance(element, ast.Constant) and isinstance(element.value, str):
139+
globs.append(element.value)
140+
if globs:
141+
return globs
142+
raise AssertionError(
143+
"conftest.py does not gate collect_ignore_glob on `not _coremltools_is_declared`, "
144+
"so the Core ML tests are collected even where coremltools is not installed"
145+
)
146+
147+
148+
def _marker_says_installed(
149+
requirement: str, system, machine, sys_platform, python
150+
) -> bool:
151+
_, _, marker = requirement.partition(";")
152+
assert marker.strip(), f"{requirement} has no environment marker"
153+
return Marker(marker).evaluate(
154+
{
155+
"platform_system": system,
156+
"platform_machine": machine,
157+
"sys_platform": sys_platform,
158+
"python_version": f"{python[0]}.{python[1]}",
159+
"python_full_version": f"{python[0]}.{python[1]}.0",
160+
}
161+
)
162+
163+
164+
def _conftest_says_installed(machine, sys_platform, python) -> bool:
165+
return bool(
166+
eval( # the expression is read out of conftest.py, not taken from input
167+
compile(_conftest_condition(), "<conftest.py>", "eval"),
168+
{
169+
"sys": SimpleNamespace(platform=sys_platform, version_info=python),
170+
"platform": SimpleNamespace(machine=lambda: machine),
171+
},
172+
)
173+
)
174+
175+
176+
def _support_helper_says_supported(system, machine, python) -> bool:
177+
"""Evaluate export/utils.py's is_supported_platform_for_coreml_lowering().
178+
179+
The function is extracted and run against stub platform/sys modules rather than
180+
imported, because importing export.utils pulls in torch and would answer for the
181+
interpreter running this test instead of for the platform under test.
182+
"""
183+
tree = ast.parse((REPO_ROOT / "export" / "utils.py").read_text())
184+
for node in tree.body:
185+
if (
186+
isinstance(node, ast.FunctionDef)
187+
and node.name == "is_supported_platform_for_coreml_lowering"
188+
):
189+
namespace = {
190+
"platform": SimpleNamespace(
191+
system=lambda: system, machine=lambda: machine
192+
),
193+
# A namedtuple-like value so both `>= (3, 14)` and `.major`/`.minor`
194+
# work, matching what sys.version_info supports.
195+
"sys": SimpleNamespace(version_info=_VersionInfo(*python)),
196+
"logging": SimpleNamespace(info=lambda *a, **k: None),
197+
}
198+
module = ast.Module(body=[node], type_ignores=[])
199+
exec(compile(module, "<export/utils.py>", "exec"), namespace)
200+
return bool(namespace["is_supported_platform_for_coreml_lowering"]())
201+
raise AssertionError(
202+
"no is_supported_platform_for_coreml_lowering in export/utils.py"
203+
)
204+
205+
206+
@pytest.mark.parametrize("system,machine,sys_platform", PLATFORMS)
207+
@pytest.mark.parametrize("python", PYTHONS)
208+
def test_conftest_matches_the_coremltools_marker(system, machine, sys_platform, python):
209+
declared = _marker_says_installed(
210+
_base_dependency("coremltools"), system, machine, sys_platform, python
211+
)
212+
collected = _conftest_says_installed(machine, sys_platform, python)
213+
assert declared == collected, (
214+
f"on {system}/{machine} with Python {python[0]}.{python[1]} setup.py "
215+
f"{'declares' if declared else 'does not declare'} coremltools while conftest.py "
216+
f"{'collects' if collected else 'skips'} the Core ML tests, so pytest will either fail "
217+
"at collection on a missing import or silently skip tests that could have run"
218+
)
219+
220+
221+
@pytest.mark.parametrize("system,machine,sys_platform", PLATFORMS)
222+
@pytest.mark.parametrize("python", PYTHONS)
223+
def test_scikit_learn_follows_coremltools(system, machine, sys_platform, python):
224+
# scikit-learn is there for coremltools' palettization, so it is only useful where
225+
# coremltools is. Nothing else in the wheel imports scikit-learn itself; scipy, which
226+
# used to arrive as its transitive dependency, is declared in requirements-examples.txt
227+
# for the code that does import it.
228+
assert _marker_says_installed(
229+
_base_dependency("scikit-learn"), system, machine, sys_platform, python
230+
) == _marker_says_installed(
231+
_base_dependency("coremltools"), system, machine, sys_platform, python
232+
)
233+
234+
235+
def test_the_platforms_core_ml_is_used_on_are_still_covered():
236+
# A guard on the parametrized tests above: they would pass just as happily if both
237+
# rules said "never", which would quietly stop running the Core ML tests everywhere.
238+
requirement = _base_dependency("coremltools")
239+
assert _marker_says_installed(requirement, "Darwin", "arm64", "darwin", (3, 12))
240+
assert _marker_says_installed(requirement, "Linux", "x86_64", "linux", (3, 12))
241+
assert not _marker_says_installed(requirement, "Linux", "aarch64", "linux", (3, 12))
242+
assert not _marker_says_installed(requirement, "Windows", "AMD64", "win32", (3, 12))
243+
244+
245+
def test_conftest_acts_on_the_condition():
246+
# The tests above only compare the two rules as expressions. They would still pass if
247+
# conftest.py computed _coremltools_is_declared and then never used it, which is the
248+
# one failure that reaches contributors as a ModuleNotFoundError at collection.
249+
#
250+
# Membership, not equality: adding a second correct glob is a legitimate change, and
251+
# test_the_ignore_globs_cover_every_core_ml_test_module below already catches a glob
252+
# that stops matching.
253+
assert "backends/apple/coreml/**" in _coreml_ignore_globs()
254+
255+
256+
@pytest.mark.parametrize("system,machine,sys_platform", PLATFORMS)
257+
@pytest.mark.parametrize("python", PYTHONS)
258+
def test_export_support_helper_never_claims_more_than_the_marker(
259+
system, machine, sys_platform, python
260+
):
261+
# export/utils.py is the third place this rule is written down. Callers use it as an
262+
# import guard (export/target_recipes.py raises ValueError when it says no), so it must
263+
# never claim support where coremltools is not declared, or the guard is bypassed and
264+
# the import raises ModuleNotFoundError instead.
265+
#
266+
# The reverse is allowed: the helper is deliberately narrower than the marker, because
267+
# coremltools installs on Darwin x86_64 but lowering is only supported on Apple silicon.
268+
declared = _marker_says_installed(
269+
_base_dependency("coremltools"), system, machine, sys_platform, python
270+
)
271+
supported = _support_helper_says_supported(system, machine, python)
272+
assert not (supported and not declared), (
273+
f"on {system}/{machine} with Python {python[0]}.{python[1]} "
274+
"is_supported_platform_for_coreml_lowering() reports support while setup.py does "
275+
"not declare coremltools, so the import guard in export/target_recipes.py is "
276+
"bypassed and importing coremltools raises ModuleNotFoundError"
277+
)
278+
279+
280+
def test_the_support_helper_still_says_yes_where_core_ml_is_used():
281+
# A guard on the one-directional test above, which a helper that always returned False
282+
# would satisfy trivially.
283+
assert _support_helper_says_supported("Darwin", "arm64", (3, 12))
284+
assert _support_helper_says_supported("Linux", "x86_64", (3, 12))
285+
assert not _support_helper_says_supported("Windows", "AMD64", (3, 12))
286+
287+
288+
def test_the_support_helper_tracks_the_marker_on_python_version():
289+
# The one-directional test above is deliberately loose on architecture, because
290+
# coremltools installs on Darwin x86_64 while lowering needs Apple silicon. It must not
291+
# be loose on the Python version too: if the marker starts declaring coremltools on a
292+
# version the helper still refuses, every caller keeps taking the "not supported" branch
293+
# on a platform where Core ML now works, and nothing else in this file notices.
294+
#
295+
# So pin the helper to the marker's own bound rather than to a literal, and read the
296+
# bound out of setup.py so widening the marker fails here until the helper follows.
297+
marker = _base_dependency("coremltools")
298+
for python in PYTHONS:
299+
declared = _marker_says_installed(marker, "Darwin", "arm64", "darwin", python)
300+
supported = _support_helper_says_supported("Darwin", "arm64", python)
301+
assert supported == declared, (
302+
f"on Darwin/arm64 with Python {python[0]}.{python[1]} setup.py "
303+
f"{'declares' if declared else 'does not declare'} coremltools but "
304+
f"is_supported_platform_for_coreml_lowering() reports "
305+
f"{'support' if supported else 'no support'}; the version bound in "
306+
"export/utils.py has drifted from the marker in setup.py"
307+
)
308+
309+
310+
def test_the_ignore_globs_cover_every_core_ml_test_module():
311+
# A glob that no longer matches would silence nothing. Checked against the files on
312+
# disk so that moving the Core ML tests without updating conftest.py fails here.
313+
coreml_tests = sorted(
314+
path.relative_to(REPO_ROOT).as_posix()
315+
for path in (REPO_ROOT / "backends/apple/coreml").rglob("test_*.py")
316+
)
317+
assert coreml_tests, "no Core ML test modules found, so this guard proves nothing"
318+
globs = _coreml_ignore_globs()
319+
for test in coreml_tests:
320+
assert any(
321+
fnmatch.fnmatch(test, glob) for glob in globs
322+
), f"{test} is not covered by conftest.py's ignore globs {globs}"

‎.wiki/backends/coreml/overview.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,4 +273,4 @@ The `to_edge_with_preserved_ops` API (experimental) allows preserving ops like `
273273

274274
## CoreML Export on Linux
275275

276-
CoreML export now works on Linux (as of v0.6+). The `coremltools` package can run on Linux for AOT compilation, though runtime execution still requires macOS/iOS. [Source: #9800]
276+
CoreML export now works on Linux x86_64 (as of v0.6+). The `coremltools` package can run there for AOT compilation, though runtime execution still requires macOS/iOS. It is x86_64 only because coremltools publishes no build for any other Linux architecture. [Source: #9800]

‎conftest.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import hashlib
2+
import platform
23
import sys
34

45
import torch
@@ -14,6 +15,22 @@
1415
"backends/apple/**",
1516
]
1617

18+
# Every test file under backends/apple/coreml that defines tests imports coremltools at module
19+
# scope, so collection fails wherever the wheel does not declare it. Windows is already covered
20+
# above; what is left is any Linux that is not x86_64, since coremltools publishes no build for
21+
# those, and Python 3.14, for which it publishes no build on any platform. Keep this condition in
22+
# sync with the coremltools marker in setup.py; .ci/scripts/tests/test_coreml_markers.py checks
23+
# that the two agree.
24+
_coremltools_is_declared = (
25+
sys.platform == "darwin"
26+
or (sys.platform.startswith("linux") and platform.machine() == "x86_64")
27+
) and sys.version_info < (3, 14)
28+
29+
if not _coremltools_is_declared:
30+
collect_ignore_glob += [
31+
"backends/apple/coreml/**",
32+
]
33+
1734

1835
def pytest_runtest_setup(item):
1936
# Set a stable seed for each test based on a hash of the test name.

‎export/utils.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# pyre-strict
88
import logging
99
import platform
10+
import sys
1011

1112
import torch
1213

@@ -20,6 +21,17 @@ def is_supported_platform_for_coreml_lowering() -> bool:
2021
system = platform.system()
2122
machine = platform.machine().lower()
2223

24+
# coremltools has no wheel for 3.14 yet, so setup.py does not declare it
25+
# there. Callers use this as an import guard, so reporting the platform as
26+
# supported would turn a handled "not supported" into a ModuleNotFoundError.
27+
# Keep this in step with the coremltools marker in setup.py.
28+
if sys.version_info >= (3, 14):
29+
logging.info(
30+
f"Unsupported Python for CoreML: {sys.version_info.major}."
31+
f"{sys.version_info.minor}"
32+
)
33+
return False
34+
2335
# Check for Linux x86_64
2436
if system == "Linux" and machine == "x86_64":
2537
return True

‎requirements-examples.txt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
# pip packages needed to run examples.
22
# TODO: Make each example publish its own requirements.txt
33
datasets == 3.6.0 # 4.0.0 deprecates trust_remote_code and load scripts. For now pin to 3.6.0
4+
# examples/openvino/aot_optimize_and_infer.py scores accuracy with scikit-learn and
5+
# examples/qualcomm/scripts/mobilebert_fine_tune.py uses it too. Three Qualcomm example scripts
6+
# import scipy directly. Both used to arrive by accident, scikit-learn as an unmarked base
7+
# dependency and scipy as its transitive, and neither was ever declared. Now that scikit-learn
8+
# follows coremltools they have to be named where they are actually used.
9+
scikit-learn >= 1.7.1
10+
scipy
411
timm == 1.0.7
512
torchsr == 1.0.4
613
# torchtune caps pyarrow below 21, and no pyarrow in that range ships a cp314 wheel,

0 commit comments

Comments
 (0)