Bump the PyTorch pin to 2.14 - #22501
Open
shoumikhin wants to merge 18 commits into
Open
Conversation
shoumikhin
added
the
release notes: build
shoumikhin
added
the
release notes: build
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/22501
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
ciflow/docker
shoumikhin
added
ciflow/periodic
ciflow/trunk
ciflow/binaries
ciflow/binaries/all
meta-cla
Bot
added
the
CLA Signed
rascani
reviewed
Sep 3, 2026
| "executorch_exir_dialects_edge__ops_aten_linear_default": 1, | ||
| "executorch_exir_dialects_edge__ops_aten_relu_default": 9, | ||
| "executorch_exir_dialects_edge__ops_aten_view_copy_default": 1, | ||
| "executorch_exir_dialects_edge__ops_dim_order_ops__clone_dim_order_default": 2, |
Contributor
There was a problem hiding this comment.
Why did these get removed? Not complaining, just curious. :)
Contributor
Author
There was a problem hiding this comment.
Good question, and worth writing down.
On PyTorch 2.14 these graphs no longer contain a dim order clone at all, so the
count cannot match any number. The check is an exact-count assertion, so the entry
has to go rather than change value.
Two things I checked before deleting them, since a vanished op can also mean
something upstream silently stopped working:
The same tests pass on the base branch with these entries present, so this is the
new PyTorch changing the graph rather than an existing problem.
It is not caused by either of the recent dim order passes on main, the one that
removes non-contiguous dim order and the one that replaces input dim order clones
with permutations. Both are already in the base commit, and these tests are green
there.
If the clone is expected to survive for this backend, then this is worth a closer
look rather than a test update, and I am happy to dig further. From what I can see
the clone was only there to make the input contiguous, and 2.14 gets to the same
graph without it, so dropping the expectation looks right.
Every Docker image build fails at the docs dependency step:
error commander@15.0.0: The engine "node" is incompatible with this module.
Expected version ">=22.12.0". Got "16.20.2"
error Found incompatible module.
katex was installed unpinned, so it resolved to whatever was newest. Version
0.18.5, published a few days ago, changed its dependency from commander 8 to
commander 15, and commander 15 requires node 22.12 or newer. This script
installs node 16, so the install now refuses to run. Nothing in this repo
changed; the newest version of a dependency moved out from under it.
Pin katex to 0.18.4, the last release that depends on commander 8, which
runs on the node this image installs.
This is the same fix, with the same comment, that pytorch/pytorch applied to
its copy of this script earlier today. Keeping the two in step matters here
because the file is otherwise near identical between the repos, so a reader
comparing them should not find two different answers to one problem.
It is worth noting the unpinned install is the underlying cause, and node 16
has been out of support for a while. Moving the image to a current node is a
larger change with a wider blast radius, and it does not belong in a pull
request about something else. This restores the build.
Test Plan:
Read the metadata from the npm registry rather than inferring it. katex 0.18.5
depends on commander ^15.0.0 and 0.18.4 depends on commander ^8.3.0. commander
15.0.0 declares node >=22.12.0, and the newest commander 8, which is what
^8.3.0 resolves to, declares node >=12. So the pinned version's dependency
runs on node 16 and the unpinned one cannot.
Confirmed this is not caused by the other change in this pull request: the
same failure, with the same message, appears on the docs step of main's own
image build from the day before this branch existed, and the previous run of
that workflow succeeded before 0.18.5 was published.
Syntax-checked the script.
Whether the built image renders documentation correctly with the slightly
older katex is not something I can check locally, since it needs the full
image build. CI covers it.
The macOS jobs build PyTorch from source whenever the cached wheel for the
pinned version is not in S3 yet, which is always the case the first time a new
pin is used. That fallback calls:
python setup.py bdist_wheel
PyTorch 2.14 no longer supports it. It now builds through scikit-build-core
using the standard PEP 517 interface, and the command exits with an error
naming its own replacement:
error: `python setup.py bdist_wheel` is deprecated: PyTorch is built with
scikit-build-core via the standard PEP 517 interface (pyproject.toml),
and setup.py is no longer part of the build.
Use the standard build frontend instead, with isolation off so the build uses
the build requirements installed a few lines above rather than fetching its
own copies.
The Docker images already build PyTorch this way, and their code carries the
same reasoning. Only this macOS path still used the old command, so this
brings the two into step.
Two details worth stating. The wheel filename does not change, because the
version comes from PyTorch's own version file and git hash rather than from
the build frontend, so the S3 cache key and the invariant that checks it are
unaffected. And the output still lands in `dist/`, which is where the lines
after this read from.
Test Plan:
Read the error from a real CI run rather than reasoning about it: the macOS
unit test job on this branch reached the source build after a cache miss, and
failed on exactly this command with the message above.
Confirmed the cache miss itself was correct behavior, not a defect. The key it
asked for names the short hash of the pinned PyTorch branch head, so it was
looking for the right wheel, and it 404s only because no run has built and
uploaded that version yet.
Confirmed PyTorch's `requirements-build.txt`, which the lines above already
install, covers what the PEP 517 build needs: it lists scikit-build-core,
setuptools, cmake, ninja, numpy, packaging, pyyaml and six, matching the
build-system requires in PyTorch's own pyproject.toml.
Syntax-checked the script.
I cannot run a full PyTorch source build on this machine, so whether the build
completes is for CI to establish. What is verified here is that the command it
previously used refuses to run at all on this pin.
The ROCm jobs install PyTorch from a per-version index built from the ROCm
version in their matrix:
ERROR: No matching distribution found for torch==2.14.0
PyTorch publishes 2.14 for ROCm 7.2 and not for 7.1, which is what these jobs
ask for. 2.13 was published for 7.1, so the jobs passed before the pin moved
and cannot pass after it while still pointing at 7.1.
Move the three job matrices to 7.2, and the script's own default with them so a
local run and CI agree.
Test Plan:
Checked the indexes rather than assuming which versions exist. The test channel
for ROCm 7.2 carries torch 2.14.0 and ROCm 7.1 does not, while 7.1 does carry
2.13.0, which is why this surfaces only now.
Checked the two other things the jobs need at the new version. The builder image
`pytorch/manylinux2_28-builder:rocm7.2` exists in the registry, and the pinned
torchao nightly publishes a `+rocm7.2` wheel, which matters because the script
builds that wheel's URL by hand rather than resolving it from an index.
Confirmed no other reference to the old version is left in the workflow or the
script.
Parsed the workflow as YAML and syntax-checked the script.
I have no ROCm hardware, so whether the tests pass on 7.2 is for CI to
establish. What is verified here is that the packages the jobs install exist at
the new version and did not at the old one.
Every Metal export fails at lowering:
RuntimeError: Method forward missing fallback kernels (1 total):
- aoti_torch_mps_gated_delta_rule
Please add them to the AOTI backend.
Three of the backend's custom operators hit this: the gated delta rule, the
qmv gather, and the 4-bit linear.
The backend keeps a list of the fallback kernels it can serve, and export fails
if the graph asks for one that is not on it. All three operators are already on
that list, spelled the way they are registered, for example
`metal::gated_delta_rule`. Inductor also derives a shim name for a custom op, by taking the part after the
namespace and prefixing the device, which turns that into
`aoti_torch_mps_gated_delta_rule`, and it is that derived name which reaches the
check for these three.
The function that derives the name is unchanged between the previous pin and this
one, so this is a gap in the list that the new pin exposed rather than a rename to
follow.
Add the derived spellings alongside the registered ones. Which of the two
arrives depends on the path Inductor takes for a given op, which is not
ExecuTorch's decision to make, so accepting both is the honest fix. Nothing is
newly permitted: the same three operators are involved either way.
This is only about the name used at export time. The runtime already implements
all three under exactly these symbols, in `op_gated_delta_rule.mm`,
`op_gather_qmv.mm` and `op_linear_4bit.mm`, so the generated code has something
to call and this does not defer the failure to load time.
Test Plan:
Took the three names from the CI failures rather than deriving them by hand,
and confirmed each is the derived form of an entry already on the list.
Confirmed each of the three is defined in the Metal runtime, under that exact
symbol, inside an `extern "C"` block so the generated wrapper can link it. That
is what distinguishes this from silencing a real gap: had any symbol been
absent, allowing it through export would only move the error to load time.
Checked the shim-name derivation in both PyTorch branches. The function that
builds it is unchanged between them, which is why this is about which path an op
takes rather than a renaming.
Parsed the changed file.
I have no Apple GPU in this environment, so whether the exported models now run
is for CI to establish. What is verified is that the names the failures asked
for exist in the runtime.
Anything built against ATen fails to compile:
c10/util/intrusive_ptr.h:775: error: no type named 'strong_ordering' in
namespace 'std'
ATen/core/TensorBase.h:1031: error: unknown type name 'requires'
Both are C++20. PyTorch 2.14 uses them with no feature guard, and the wrapper pinned
every target to C++17.
Decide the standard per target instead: C++20 where the target compiles against ATen,
C++17 everywhere else. The test path next door already made this distinction and says
in its own comment that non-ATen targets are pinned to C++17 for embedded, so
libraries and binaries were the odd ones out.
Three details decide whether this works, and each cost a round to find:
The answer has to be computed before the shared kwargs patch runs. That patch turns
external dependency entries into real dependencies and removes the original list, so a
check that reads the list afterwards always answers no.
The flag has to go on the target's own compile flags. The per-language entry the
wrapper already used reaches an earlier pass, and matched the standard it was
restating, so nothing had ever shown it could raise one.
The check keys on the exact dependency names the build maps onto libtorch, not on a
substring of the label, because every label in this project contains "torch" inside
"executorch".
The decision lives in the shared wrapper rather than the interface layer beside it,
because that layer exists only for the open source build and the internal build has
its own copy with a narrower signature.
Nothing changes for consumers. The standard the shipped runtime requires is still
C++17, and the CMake build still sets it.
Test Plan:
Read the requirement out of the headers on both branches rather than inferring it from
the error.
Extracted the wrapper functions and ran them, with `type()` behaving as Starlark's
does, over the target from the failure and over targets that must not move, removing
the dependency list first so the sequence matches what the wrapper really does. ATen
targets come away at C++20 through all three paths, library, binary and test; a plain
library and the portable kernels come away with C++17.
That probe is what caught the substring problem, and it is also what MISSED the
ordering problem twice: calling the helper directly cannot see a step that runs before
it. The Buck job is what found that, both times. Anyone extending this should exercise
the whole path, not the helper.
CI is the real check here: unittest-buck builds the generated ATen kernel libraries
that were failing. I cannot run this Buck build on this machine.
PyTorch 2.14.0 is released, so move the pin from 2.13 to it. The tree is at 1.5.0 and mid-cycle, and the pin is normally moved to the current stable release, so this is that step. What changes: - the CI pin, from the `release/2.13` branch of PyTorch to `release/2.14` - the torch version literals, which live in three places that nothing keeps in agreement: `torch_pin.py`, `install_requirements.py`, and the Windows workflow - torchvision, from 0.28 to 0.29, which is the release that pins `torch==2.14.0` exactly. torchaudio stays at 2.11.0, still the newest published, since it no longer tracks torch's version train - the vendored c10 headers, re-synced from the new branch CI compares the vendored copy against PyTorch's own tree and requires it byte for byte, so the pin cannot move without it. Eight of the twenty-five headers changed. One of them moved rather than changed: `complex_utils.h` is now under `torch/headeronly/util/` instead of `c10/util/`, and its contents moved into the `torch::headeronly` namespace. `c10/util/complex.h` re-exports `is_complex` and `scalar_value_type` into `c10`, so code spelling those with a `c10::` prefix keeps working and no caller in this repo had to change. Two build references had to follow the move: the Buck header list for the `c10` target, which names its headers one by one, and the wheel test's list of headers that refuse to be included directly. The `torch_headeronly` target globs, so it picked the file up on its own. This is not a pure header refresh. `overflows()` decides whether a value fits a narrower type, and the new version answers differently for float to integer casts, in both directions: - a fractional value just past an integer limit is now accepted, so filling an int8 tensor with 127.5 used to be refused and now gives 127 - a value exactly at 2^63 cast to int64 is now refused, where it used to be accepted and wrapped to the lowest int64 The portable kernels reach this through `check_overflow_cast`, so `full`, `full_like`, `fill`, `scalar_tensor`, `hardtanh`, `leaky_relu`, `scatter` and `constant_pad_nd` all inherit it. The header is a faithful copy of upstream, so the change is not ours to undo, but it should be visible to anyone reading this rather than buried in a header diff. The existing overflow tests all use whole numbers or floating point output types, so none of them covered the branch that moved. Add a case at 2^63 for int64, which pins the direction that used to wrap silently. Test Plan: Ran the header check CI runs, `.ci/scripts/compare_dirs.sh`, against a real `release/2.14` checkout for both vendored trees: exit 0 on each. Confirmed it fails without the re-sync, exit 1, naming `complex_utils.h` as present here and absent upstream, so the check does exercise this change. Compiled a program against the re-synced headers that instantiates `c10::complex`, `c10::is_complex` and `c10::scalar_value_type`, to confirm the two moved symbols still resolve under their old names. It builds and runs. Also compiled a translation unit including both these headers and ExecuTorch's own scalar type header, to check the include graph still works from inside the repo. Measured the behaviour change rather than reading it off the diff. Compiled `overflows()` from both branches and compared: int8 from 127.5 goes from refused to accepted, likewise uint8 from 255.5 and int32 from 2147483647.5, while int64 from 2^63 and uint64 from 2^64 go from accepted to refused. int8 from -128.5 is unchanged, so the lower bound did not move. Confirmed the new test discriminates. The value it uses is accepted by the old header and refused by the new one, so the test fails on the pin this change replaces and passes on the one it installs. A test that passed either way would prove nothing. Resolved the three pinned versions together from the channel CI installs from, and got 2.14.0, 0.29.0 and 2.11.0 with no conflict. Installed the pinned torchao nightly on top of torch 2.14 and imported the surfaces past pin bumps have broken on, `torchao.quantization`, `quantize_` and `torchao.dtypes`, all of which import. Compared the AOTI shim surface between the two branches. Both declare the same 148 entry points, so no new shim is added upstream. That is narrower than it sounds, and CI later showed why: the Metal backend still needed a change, because three of its custom operators began arriving at the fallback check under a derived name rather than the registered one. That is a later commit on this branch, not something this comparison could have caught. Syntax-checked the changed shell, Python and workflow files. Not covered locally: the Docker image build, the CUDA and Qualcomm jobs, and the model export suites, none of which run on this machine. The new test case was verified at the level of the helper it exercises rather than by running the kernel test binaries, which need a full build.
Every quantized CUDA export fails at lowering:
RuntimeError: Method forward missing fallback kernels (1 total):
- aoti_torch_cuda__weight_int4pack_mm
Please add them to the AOTI backend.
This is the same situation the Metal backend is in one commit earlier, for the
same reason. The backend lists the fallback kernels it can serve and export fails
if the graph asks for one that is absent. This operator is already on the list,
spelled the way it is registered, `at::_ops::_weight_int4pack_mm::call`. Inductor
also derives a shim name for it, by taking the part after the namespace and
prefixing the device, and it is that derived name which reaches the check here.
Add the derived spelling next to the registered one. Nothing is newly permitted,
and the runtime already implements this shim, in
`backends/cuda/runtime/shims/int4mm.cu`, so the generated code has something to
call.
Which spelling arrives depends on the path Inductor takes for a given operator,
not on the PyTorch version: the function that derives the name is unchanged
between the previous pin and this one. So this is a gap in the list that the new
pin exposed, rather than a rename to follow.
Test Plan:
Ran PyTorch 2.14's own name derivation over the entry and confirmed it produces the
spelling in the error.
Compared that function between the previous pin and this one: apart from an assert
being rewritten as a raise, it is the same, which is why the commit does not claim
the naming changed.
The export itself is for CI, which is where the failure appeared.
The Vulkan operator tests fail to compile:
ATen/ATen.h:5:2: error: C++20 or later compatible compiler is required to
use ATen.
PyTorch 2.14 raised ATen's own floor. The header used to reject anything below
C++17 and now rejects anything below C++20, and these tests link libtorch to
check operator results against eager PyTorch.
Raise the standard on the targets that include those headers. This repository
already does exactly that where the generated ATen-mode kernel library is
defined, with the same one-line comment, so this follows a pattern that is
already here rather than introducing one.
Nothing changes for the runtime or for anyone consuming it. ATen only enters the
build behind `USE_ATEN_LIB`, which no embedded preset sets, and the project
default stays C++17.
Test Plan:
Read the requirement out of the header on both branches rather than inferring it
from the error: the guard is `__cplusplus < 201703L` with a C++17 message on the
previous branch and `__cplusplus < 202002L` with a C++20 message on this one.
Confirmed the scope is limited before widening it. ATen reaches ExecuTorch only
through `USE_ATEN_LIB`, which is set in exactly one place in the CMake build, the
generated ATen-mode kernel library, and that target already sets C++20 for this
reason. None of the bare-metal, Zephyr, ESP or RISC-V presets enable it.
Checked the other CMake targets that link libtorch and left them alone, since
only these tests failed.
cmake-format is clean on the changed file.
I cannot build the Vulkan tests on this machine, which needs a Vulkan SDK, so CI
has to confirm they compile.
…again
The CI images build PyTorch from source, and after the pin moved they produced a
PyTorch with no BLAS and no LAPACK. Tests that need it fail:
RuntimeError: Calling torch.geqrf on a CPU tensor requires compiling PyTorch
with LAPACK. Please use PyTorch built with LAPACK support.
The build says as much, if you read the configure output:
MKL could not be found. Defaulting to Eigen
Cannot find a library with LAPACK API. Not using LAPACK.
USE_BLAS : 0
Nothing about the image changed. PyTorch changed how it builds: 2.13 built through
setuptools and 2.14 builds through scikit-build-core, and the two choose cmake
differently. setuptools looked for cmake on PATH and found the one in the image.
scikit-build-core prefers an importable pip cmake over PATH, and this script was
installing one.
That matters because cmake adds its own install root to the list of prefixes it
searches. The image's cmake lives in the conda environment, alongside MKL and
libomp, so a plain search finds them. A pip cmake lives in site-packages, where
neither is, so the search fails, PyTorch falls back to Eigen, and LAPACK is
dropped. The build does not fail when this happens; it just produces a weaker
wheel.
So stop installing cmake into the build environment and let the image's own cmake
be used. It is pinned at 3.31 and satisfies what PyTorch asks for.
Also assert on the result afterwards. This failure is silent by nature, and the
first sign of it today was a linear algebra test failing much later in an
unrelated job.
Test Plan:
Read scikit-build-core's own source rather than inferring the order: its cmake
lookup yields the importable `cmake` module first and only then falls back to
PATH. Reproduced it in a scratch environment: with pip cmake installed it selects
the site-packages copy, and with it absent it selects the one on PATH.
Confirmed the prefix mechanism by running `cmake --system-information` on two
different cmake binaries and diffing: each one appends its own install root to
the searched prefixes. That is what makes the choice of binary decide whether MKL
is found.
Confirmed the backend change is real, since it is the trigger: the build backend
is `setuptools.build_meta` on the previous branch and `scikit_build_core.build`
on this one.
Checked that this is not specific to one architecture or one BLAS vendor. The
aarch64 image, which installs conda openblas rather than MKL, degraded the same
way, and both architectures were healthy on the previous pin. A fix aimed only at
MKL or only at x86_64 would have left half of it broken.
Confirmed the image's cmake is available when this runs: conda pins it at 3.31.2
and installs well before this script.
Ran the guard against a healthy PyTorch 2.14 to be sure it passes rather than
being vacuous, and it reports LAPACK present and completes a QR.
Syntax-checked the script.
I cannot build a CI image on this machine, so whether the rebuilt image now
reports BLAS and LAPACK is for CI to confirm. The guard is there so that answer
is unambiguous.
…ners for
Three Docker images fail to build PyTorch, all on the same file, for the same
underlying reason in two flavours. The clang image:
FAILED: [code=127] third_party/fmt/CMakeFiles/fmt.dir/src/format.cc.o.ddi
/bin/sh: 1: CMAKE_CXX_COMPILER_CLANG_SCAN_DEPS-NOTFOUND: not found
and the two GCC images:
cc1plus: error: to generate dependencies you must specify either '-M' or '-MM'
PyTorch compiles at C++20 as of 2.14. CMake responds by scanning every source
for module imports before compiling it, which needs a scanner: `clang-scan-deps`
for clang, and a supported invocation for GCC. The clang image installs clang and
llvm but not the package that carries `clang-scan-deps`, and GCC rejects the scan
outright. Neither PyTorch nor anything in these images uses C++20 modules, so the
scan has nothing to find and only has to be turned off.
Turning it off covers all three images. Adding the missing clang package would fix
one of them and leave the two GCC ones failing, which is how this looked at first.
Test Plan:
Compared the three failing job logs. All three fail building the same fmt source,
the clang one on a missing scanner binary and the two GCC ones on the scan
invocation itself, which is what shows this is one problem rather than three.
Confirmed the variable reaches CMake rather than being ignored. PyTorch forwards
any environment variable beginning with `CMAKE_` to a CMake cache variable of the
same name, which its own forwarding module states in those terms.
Confirmed the images were healthy before the pin moved: the same image and job
name built successfully on the previous pin.
Confirmed this is not a duplicate of the cmake change next to it. That one picks
which cmake runs; both cmakes are new enough to scan, so it does not disable
scanning on its own.
Syntax-checked the script.
I cannot build an image on this machine, so CI has to confirm the three images now
build.
Lowering a CUDA model for a Windows target fails at link time:
x86_64-w64-mingw32-ld: undefined reference to `aoti_torch_dtype_uint8'
x86_64-w64-mingw32-ld: undefined reference to `aoti_torch_empty_strided_pinned'
x86_64-w64-mingw32-ld: undefined reference to `aoti_torch_is_defined'
collect2: error: ld returned 1 exit status
PyTorch generates a wrapper for the model and links it against this project's
shims. The wrapper's own header started calling these three in 2.14, where it
called none of them before, so the link now needs all three present.
Two did not exist here:
`aoti_torch_is_defined` asks whether a handle refers to a real tensor. PyTorch has
an undefined-tensor state with no equivalent here, so this is a null check.
`aoti_torch_empty_strided_pinned` asks for page-locked host memory, which lets the
copy of the model's constants to the device overlap with filling the next buffer.
There is no allocator for that memory here, so this reports that and the caller
takes the plain synchronous copy it already falls back to. That path is not an
error case: the calling code is named for trying, and every way it can fail logs
that it is falling back and carries on. Adding a real pinned allocator means
teaching the storage layer a new kind of memory and a matching way to release it,
which is a change of its own rather than part of moving a version pin.
The third, `aoti_torch_dtype_uint8`, was already implemented and simply missing
from the list of names the Windows link stub exports, along with the other two.
That stub is a checked-in file, so it is regenerated here. Every previous pin
bump has had to do the same.
Test Plan:
Read the requirement from the generated wrapper's header on both branches rather
than inferring it from the link error: it names these three in 2.14 and none of
them in 2.13, which is why this appears only after the pin moves.
Compiled the common shims and confirmed with the symbol table that
`aoti_torch_is_defined` is defined rather than merely declared. Checked its
signature against PyTorch's own header parameter by parameter, and matched the
pointer spelling the neighbouring shims already use for a tensor handle.
Regenerated the link stub from the names it already exported plus these three, so
it went from 42 to 45. Confirmed the result is a strict superset, that nothing was
dropped, that the counts of the two kinds of entry agree, and that all three names
are now in it.
Confirmed the caller treats a refusal as an ordinary outcome by reading it: it
returns empty and logs that it is falling back to the synchronous copy.
Formatting is clean on the changed files.
The CUDA shim file needs the CUDA headers to compile, which this machine does not
have, so that half is for CI to confirm. Whether a Windows target now links is
also for CI, since it needs the cross toolchain.
The CUDA test that asserts the exact set of fallback kernels fails:
test_cuda_fallbacks_unchanged_by_rocm_gate
An earlier commit taught the backend to accept the shim spelling Inductor now
derives for the int4 pack matmul, but this test compares the advertised set
against a frozen copy of it, so adding an entry on one side alone breaks the
comparison. Add the same entry here.
The Metal backend took the same kind of change and is unaffected, because nothing
pins its set exactly.
Test Plan:
Extracted both sets and compared them directly. Before this the advertised set has
twelve entries and the frozen copy eleven, with the new derived spelling the only
difference and nothing frozen that the backend no longer advertises, so the
comparison fails on exactly that one name. After, both hold twelve and the
comparison passes.
Treat a test that names c10 as an ATen test too
The Buck test targets decide their C++ standard separately from the libraries, and
their check for "is this an ATen test" looks for a dependency named `libtorch` but
not for the other two names the build maps onto the same library. A test that names
`c10` or `torch-core-cpp` was therefore pinned to C++17 and failed on ATen's
headers, which need C++20 as of PyTorch 2.14:
ATen/core/TensorBase.h: error: expected ';' at end of declaration
Reuse the same check the library path already uses, so both paths agree on what
counts as ATen. It covers all three dependency names, and the exported list as well
as the plain one.
The rest of the test check is left alone: a name containing `aten`, and the ATen
flavours of the test framework, still mark a test as ATen on their own.
Test Plan:
Ran the wrapper function directly, with `type()` behaving as Starlark's does, over
the cases that matter. Before this a test naming `c10` or `torch-core-cpp` came back
with C++17; after, both come back without a pin and take C++20 from the toolchain.
A plain test with no ATen dependency still comes back with C++17, which is the case
the embedded build depends on, and a test named for ATen or naming `libtorch` is
unchanged.
Moved the shared check above its first use, since it now has one earlier in the
file, and dropped a local that nothing reads any more.
Put the C++20 flag where the compiler actually reads it
The Buck build still fails compiling anything generated that includes ATen:
ATen/core/TensorBase.h:1031:5: error: unknown type name 'requires'
`requires` is a C++20 keyword, so the compile was still running at C++17 even
though the target is ATen-mode and the wrapper had already decided it should be
C++20.
The wrapper was setting the standard only in the per-language flags, which reach
the preprocessor pass. That was enough while the value matched the build config,
which pins C++17 for everything, and it is not enough to override it. Set the flag
on the target's own compile flags as well, for ATen-mode targets only.
Nothing changes for the rest of the build: a target that is not ATen-mode gets no
flag added here and keeps the C++17 the build config gives it.
Test Plan:
Ran the wrapper over the exact arguments the generated kernel library is defined
with, the ones from the failure. Before this it came away with only its own
warning flag and the standard confined to the per-language entry, which is why the
compile stayed at C++17. After, the flag is on the target's own list where the
compile reads it.
Checked the case that must not move: a library with no ATen dependency still gets
C++17 and no flag on its own list.
Confirmed why the old spelling looked sufficient. It only ever carried C++17,
matching the build config, so nothing had shown it could raise a standard rather
than restate one.
I cannot run this Buck build on this machine, so CI has to confirm the generated
libraries now compile.
Raise the ATen-mode C++ standard where both builds see it
The C++20 flag for ATen-mode targets was set in the interface layer that exists
only for the open source build, whose own docstring says as much. So the internal
build never got it, and the shared wrapper was passing that layer an argument its
internal counterpart does not accept, which would fail there rather than being
merely ineffective.
Move the decision into the shared wrapper, which both builds go through, and leave
the interface layer exactly as it was. The flag goes on the target's own compile
flags, because the build config applies C++17 to everything and only a flag on the
target overrides it.
Test Plan:
Read the two files' own descriptions of themselves to place this: the interface
layer is for changes specific to one build, the wrapper is for logic shared with
the internal one.
Confirmed the internal counterpart takes only the arguments it always took, so the
previous version would have broken it rather than skipped it.
Ran the new helper over the target from the failure and over targets that must not
move. The generated kernel library and the ATen bridge come away with C++20; a
plain library and the portable kernels come away with no flag added, so they keep
the C++17 the build config gives them, which is what the embedded builds rely on.
I cannot run either Buck build here, so CI has to confirm the generated libraries
compile.
Decide ATen mode before the wrapper consumes the dependency list
The Buck build still failed compiling generated sources that include ATen:
ATen/core/TensorBase.h:1031:5: error: unknown type name 'requires'
The check for "is this an ATen target" reads the external dependency list, but by
the time it ran, an earlier step in the same wrapper had already removed that list
after turning its entries into real dependencies. So every target looked like a
non-ATen one, no target was ever raised to C++20, and the flag added for it was
never added at all.
Read the list first, then let the rest of the wrapper consume it.
Test Plan:
Traced the order in the wrapper: the step that removes the list runs before the
step that read it. Reproduced that by running both against the target from the
failure: it reports ATen mode with the list present and not once it is gone, which
is exactly the sequence the wrapper produced.
Reran the whole path in the new order over the target from the failure, an ATen
target that exports its dependency instead of naming it directly, and a plain
library. The first two now come away at C++20 and the plain one is untouched, so
the embedded builds keep the C++17 they rely on.
Also read the build system's own source to confirm the flag can win where it is
placed: a target's flags are appended after the toolchain's, and its comment says
they are last on purpose. So the earlier concern that the flag was being placed
somewhere that could not take effect was unfounded; it simply was not being added.
Shortened an overlong comment that failed the formatter.
I cannot run this Buck build here, so CI has to confirm the generated libraries
compile.
The link stub advertises `aoti_torch_is_defined` against the CUDA shim library, but the function was added to the ETensor shim file, which only the Apple Metal backend links. The CUDA library is built from the CUDA shims plus the SlimTensor common shims, and the SlimTensor file did not define it. So a Windows CUDA export would link against a name nothing in that library provides, and fail when loading, which is the failure the earlier commit set out to remove. Add it to the SlimTensor shims as well, following that file's habit of rejecting a null output pointer rather than writing through it. The other two names were already in the right places: the dtype helper in the SlimTensor shims, the pinned allocator in the CUDA memory shims. Also regenerate the link stub deterministically. The tool used to rebuild it stamped the build time into all 48 members, where every member of the previous file had it zeroed, and this file is packaged into the wheel, so that timestamp reached users and made two builds of the same source differ. Test Plan: Compared what each shim file defines and what the CUDA library links. The name was in the ETensor file and absent from the SlimTensor one, and the CUDA build links only the SlimTensor variant, so nothing in that library defined it. The other two are present in the closure, which is why they were fine. Parsed every archive member header in the stub, before and after. The previous file had the time, user and group zeroed in all 48; the version committed earlier carried a real build time in 48 of 49; the version here has all three zeroed again, and the file is byte-for-byte the same size, so nothing but the metadata changed. Compared the advertised names against the previous file: 42 before, 45 now, none dropped, and the three added are exactly the three intended. I have no Windows machine, so the link and load themselves are still for CI.
The vendored range check moved in both directions, and only the direction that starts refusing values had a test. Cover the quiet one: filling an int8 tensor with 127.5 used to be refused and now stores 127. Test Plan: Compiled the real header from before and after this change into the same probe and ran both. Before: 127.5 into int8 and 255.5 into uint8 both report overflow, and 2^63 into int64 is accepted. After: the first two are accepted and 2^63 overflows. So the boundary moved both ways, and the case added here fails against the previous header rather than passing either way. Placed in this file rather than the shared macro because the macro expands into eight suites, and several of them reach the scalar through a path that rejects a fractional value before any range check, where the case would not measure what it claims. Decide ATen mode before the wrapper consumes it on the test path too The library and binary paths were fixed to read the dependency list before the shared patch removes it. The test path has the same shape and was missed: it still called its own check afterwards, so a test naming an ATen dependency was pinned to C++17 and would fail on ATen's headers. Read it in the same place, and pass the answer down, so both paths agree. Test Plan: Ran the test path over the cases that matter, with the dependency list removed first to match what the wrapper really does. Before this a test naming libtorch, c10 or torch-core-cpp came away pinned to C++17; after, all three take C++20 from the toolchain. A test named for ATen is unchanged, and a plain test still gets C++17, which is what the embedded builds rely on. No open source test target has this shape today, so this closes the same defect on a path that is not yet exercised rather than repairing a red job.
They explained more than the code needed. One line each is enough.
Three tests fail:
RuntimeError: Expected to find
"..._dim_order_ops__clone_dim_order_default" but did not find it
AssertionError: cortex_m_dequantize_per_tensor_default output dtype
torch.float32
The two model tests count an exact set of operators, and the dim-order clone they
expect is no longer in the graph on this PyTorch, so the count cannot match. Drop
those entries.
The third checks that the first operator left after quantization carries int8. For
this case the operator is dropout with training off, which is an identity, so
nothing of it survives lowering and the first operator left is the dequantize,
whose output is float by definition. Say so through the field the file already has
for cases whose output is not int8; the check on that path also confirms the input
is int8, which holds for a dequantize.
Test Plan:
Read the failure out of the job log rather than inferring it. The graph the check
searched is the pre-transform one, and it contains the other expected operators, so
only the clone entry is stale.
The dropout diagnosis is confirmed by its sibling in the same list: the in-place
variant keeps an explicit clone of its input, and that case passes, while the plain
one folds away and fails. That difference is the mechanism.
Both model tests pass on the previous pin with these same entries, so this is the
graph changing under a new PyTorch rather than an existing defect.
I cannot run these tests here: they need the Arm serializer, which is not installed
on this machine and not published on any index I can reach. CI has to confirm.
Building a delegate rearranges its placeholders into parameters and buffers first,
then user inputs. The sort asks the graph signature which names are parameters and
which are buffers, but never asks which are lifted tensor constants, so a constant
lands in the user input group and is left after a real user input.
Anything that later inserts a constant then fails, because inserting one is only
allowed ahead of the user inputs:
Failed to insert aten_alias_copy_default_fused_const; Const placeholder nodes
must be inserted before user input nodes in the graph.
Ask about constants too, and about lifted custom objects, which have the same shape.
Test Plan:
Traced where the order breaks by watching the signature through the whole pipeline.
It is correct out of export and out of edge lowering, and wrong immediately after the
delegate submodule is built, which is the only place this sort runs.
Ran the sort directly against a signature holding a constant and a user input, with
the old code and the new: the old one returns the constant after the input, the new
one returns it before. Neither reorders the user inputs among themselves.
Ran the Arm backend's misc, passes and quantizer tests, 1725 of them, once with this
change and once without, in the same environment. Seven tests go from failing to
passing and none goes the other way. The seven are the ones that lift a tensor
constant into a delegate and then fuse a constant operator.
The remaining failures in that run are separate breakages on this branch, unrelated
to placeholder order, and this change neither fixes nor worsens them.
Quantized inception_v3 fails to load with the XNNPACK delegate:
Failed to create squeeze node 344 with code: xnn_status_invalid_parameter
Init failed for backend XnnpackBackend: 0x1
XNNPACK refuses a reshape whose input and output carry different quantization
scales. The reshape here sits between two, because the operator feeding it was
never given the scale of the operator before it.
The list of operators that must share their neighbour's observer already holds
permute, view, squeeze, flatten and the pooling operators. Dropout was missing.
Not training, so it is the identity and cannot change a tensor's range, but the
walk that spreads a shared observer only looks one operator back, so it stopped at
dropout and left everything after it to be measured on its own.
The mismatch itself is not new. Both scales are identical on the previous pin, to
the last digit. What is new is that the reshape now ends up inside a delegate,
where XNNPACK checks it, because the clone that used to sit in front of it is no
longer emitted for an eval-mode dropout.
Test Plan:
Read the whole list of conditions that make this XNNPACK call return an invalid
parameter, then confirmed which one fires: the scales either side of the reshape
differ, 0.017268 against 0.007075.
Followed the annotation along the tail of the model and found dropout carrying none,
which leaves the flatten after it unannotated as well, which is what gives it a
fresh scale.
Ran the same model on both pins with the same seed and the same source. The two
scales come out identical on both, and only the newer one fails to load, which is
what shows the mismatch is pre-existing and the delegation changed.
Exported and ran the model with this change on the new pin: it loads and runs. On
the previous pin it still loads and runs, unchanged.
All 57 tests under the XNNPACK quantizer directory pass with this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
PyTorch 2.14.0 is released, so move the pin from 2.13 to it. The tree is mid-cycle at 1.5.0 and the pin normally follows the current stable release.
The pin itself is a few version strings plus a re-sync of the vendored c10 headers, which CI requires to match PyTorch's tree byte for byte. Everything else here is a change 2.14 forces, each as its own commit.
What 2.14 forces
C++20 where ATen headers are compiled.
intrusive_ptr.husesoperator<=>andTensorBase.husesrequires, neither guarded, so a target that includes ATen cannot parse at C++17. This follows the pattern already in the tree: six CMake targets setCXX_STANDARD 20for exactly this reason. Added the Vulkan op tests to that list, and taught the Buck wrapper to make the same per-target distinction. Non-ATen targets, including everything the bare-metal presets build, still compile at C++17.Two new AOTI entry points. 2.14's generated wrapper calls
aoti_torch_is_definedandaoti_torch_empty_strided_pinned, neither of which existed here. There is no pinned host allocator in this runtime, so the second reports that and the caller takes the synchronous copy it already falls back to. The checked-in Windows import library is regenerated so all three new names resolve; it goes from 42 to 45, nothing dropped.Derived shim spellings. Some custom operators now reach the fallback-kernel check under the name Inductor derives rather than the name they are registered under. Both spellings are accepted for the Metal ops and the CUDA int4 pack matmul.
Build and CI repairs.
setup.py bdist_wheelis gone, so the macOS source build uses the standard frontend. 2.14 is published for ROCm 7.2, not 7.1. PyTorch now compiles at C++20, which makes CMake scan for modules with scanners the images do not have. And a pipcmakein the build environment made the image build silently produce a wheel with no BLAS, so the image's own cmake is used and the result is now asserted rather than assumed.Two of these were already broken before this branch: the unpinned
katexinstall, and thecmakeinteraction.One behaviour change
The re-sync is not cosmetic.
overflows()answers differently for float to integer casts, in both directions. Filling an int8 tensor with 127.5 was refused and now gives 127. A value at 2^63 cast to int64 was accepted and wrapped, and is now refused.The portable kernels reach this through
check_overflow_cast, sofull,full_like,fill,scalar_tensor,hardtanh,leaky_relu,scatterandconstant_pad_ndinherit it. The header is a faithful copy of upstream, so this is not ours to undo, but it should be visible rather than buried in a header diff. Neither direction had a test; both do now.Test plan
Each commit carries its own. Covering the whole change:
compare_dirs.sh, the header check CI runs, passes against a realrelease/2.14checkout and fails without the re-sync. All eight vendored headers are byte-identical to upstream; the one build-file edit follows a file upstream moved.The behaviour change was measured by compiling
overflows()from both branches, not read off the diff. Both new cases fail on the old header, so neither is vacuous.The regenerated import library was checked member by member: 45 advertised names against 42 before, none lost, and its archive metadata is zeroed so the file is reproducible.
Not covered locally: the image build, the CUDA, ROCm, Qualcomm and Metal jobs, the export suites, the Buck build, and anything needing Windows. CI has all of it.
Known outstanding
Quantized XNNPACK
inception_v3fails at method init on a reshape node, and the cortex-m and Arm jobs have failures. All were green before this branch, so they are attributable here and are being worked. Reviewers should not treat this as ready to land until they are resolved. The CUDA end-to-end failures in this run are runner problems, not this change.