[NDTensors] Unvendor TypeParameterAccessors#1729
Merged
Merged
Conversation
Replace the vendored TypeParameterAccessors snapshot (in
`NDTensors/src/vendored/`) with a real package dependency on
`TypeParameterAccessors` v0.4.23. Removes the standalone
`module Vendored` wrapper in `NDTensors.jl` and the
`module Vendored ... include(...)` blocks in each
`NDTensorsXxxExt`, and switches every `using .Vendored.TypeParameterAccessors`
import to the registered package across `src/`, `ext/`, `lib/`, and tests.
Two API adjustments are required by upstream's redesigned
`similartype`:
1. `set_indstype(::Type{<:Tensor}, ::Tuple)` — upstream no longer
exports `set_indstype`. The single-site overload + single-site
call inside `similartype(::Type{<:Tensor}, ::Tuple)` is folded
by inlining the body directly.
2. Wrapper-aware `similartype` for arbitrary `AbstractArray`s —
upstream's fallback uses `Base.promote_op(similar, ...)`, which
gives `Any` for wrapper types (`SubArray`, `ReshapedArray`,
`Adjoint`, ...) where `Base.similar` is not type-stable, and
recurses to `Union{}` for non-wrapped arrays whose `similar` is
overloaded by NDTensors.
Defining the missing fallback as overloads on
`TypeParameterAccessors.similartype(::Type{<:AbstractArray}, ...)`
would be type piracy. Instead, this PR introduces an NDTensors-owned
helper `array_similartype` (in `abstractarray/similar.jl`) that
reproduces the trait-based wrapper-unwrap dispatch the vendored
copy provided. The helper is used at the small number of call
sites that pass a non-NDTensors-owned `AbstractArray` type to
`similartype`: NDTensors's `similar(::Type{<:AbstractArray}, ...)`
internals, `tensorstorage/similar.jl`'s `similartype` overloads
(recursing into `datatype(...)`), and `dense/dense.jl`'s
`promote_rule` for `Dense`. NDTensors-owned types continue to use
`TypeParameterAccessors.similartype` directly via their existing
overloads (which are not piracy since the types are NDTensors's).
Verified locally: `Pkg.test("NDTensors")` passes with 2845
passing, 0 failing, 4 broken (pre-existing unrelated JLArray
`@test_broken` items in `Expose/test/runtests.jl`).
Patch bump: NDTensors 0.4.26 -> 0.4.27.
The wrapper-aware `array_similartype` introduced in the previous
commit treats `Type{<:Tensor}` and `Type{<:TensorStorage}` as
wrapped arrays (their `parenttype` differs from themselves), and
the trait-based fallback unwraps them down to a plain `Array`.
That breaks the call site at
`abstractarray/similar.jl:similar(::Type{<:AbstractArray}, ::Type, ::Tuple)`,
which is invoked by
`empty/EmptyTensor.jl:_fill!!(::EmptyTensor, ...)` to materialize
an `EmptyTensor` into a `DenseTensor` on first assignment. With
the unwrap, the materialization returns a plain `Array` instead
of a `Tensor`, leaving the ITensor's `tensor` field
mistyped — surfaces as
`MethodError: no method matching _contract!!(::Array, ::Tensor, ::Tensor)`
on subsequent contractions.
Fix:
1. Add explicit `array_similartype` overloads on `Type{<:Tensor}`
and `Type{<:TensorStorage}` that delegate to
`TypeParameterAccessors.similartype`, which has NDTensors's own
overloads for these types.
2. Add the previously missing 3-arg
`similartype(::Type{<:Tensor}, ::Type, ::Tuple)`, composing the
2-arg eltype and dims forms (this was the form vendored TPA
provided generically as a wrapper-aware fallback).
Manually verified the original failure is resolved: an
`EmptyTensor`-backed `ITensor` materializes correctly to a
`DenseTensor` after `R .= NaN; R .= A .* B`. NDTensors `Pkg.test`
still passes 2845/2845.
…lartype The MappedArrays extension overloads `NDTensors.similartype` (= `TypeParameterAccessors.similartype`) to short-circuit `AbstractMappedArray` types into a plain `Array` rather than recursing through the trait-based wrapper-unwrap. Those overloads fired in NDTensors's `similar` paths under the vendored TPA setup because every recursion went through `similartype`. The previous unvendoring commits route the recursive `AbstractArray` lookups through `array_similartype` (the NDTensors-internal wrapper-aware helper) instead. The MappedArrays-specific overloads on `similartype` therefore no longer fire on those paths, and `ReadonlyMappedArray` falls into the trait-based fallback, which doesn't return a useful concrete type and breaks contraction allocation. Mirror the two relevant `similartype` overloads on `array_similartype`. Verified locally: `test/ext/NDTensorsMappedArraysExt/runtests.jl` now passes 7/7.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1729 +/- ##
=======================================
Coverage 81.09% 81.09%
=======================================
Files 59 59
Lines 4676 4676
=======================================
Hits 3792 3792
Misses 884 884 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Mechanical rename. The previous name read as "similartype for
arrays", which is a tautology (`similartype` is already an
array operation). The new name describes the distinctive
behavior: this version of `similartype` recursively unwraps
wrapper types (`SubArray`, `ReshapedArray`, `Adjoint`,
`AbstractMappedArray`, ...) via `IsWrappedArray` /
`unwrap_array_type`, where upstream's `Base.promote_op`-based
fallback would either return `Any` (because `Base.similar` is
not type-stable for the wrapper) or recurse to `Union{}`.
ipasichnyk
pushed a commit
to ipasichnyk/ITensors.jl
that referenced
this pull request
May 25, 2026
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
1 participant
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.
Replace the vendored TypeParameterAccessors snapshot (in
NDTensors/src/vendored/) with a real package dependency onTypeParameterAccessorsv0.4.23. Removes the standalonemodule Vendoredwrapper inNDTensors.jland themodule Vendored ... include(...)blocks in eachNDTensorsXxxExt, and switches everyusing .Vendored.TypeParameterAccessorsimport to the registered package acrosssrc/,ext/,lib/, and tests.Two API adjustments are required by upstream's redesigned
similartype:set_indstype(::Type{<:Tensor}, ::Tuple)— upstream no longer exportsset_indstype. The single-site overload + single-site call insidesimilartype(::Type{<:Tensor}, ::Tuple)is folded by inlining the body directly.Wrapper-aware
similartypefor arbitraryAbstractArrays — upstream's fallback usesBase.promote_op(similar, ...), which givesAnyfor wrapper types (SubArray,ReshapedArray,Adjoint, ...) whereBase.similaris not type-stable, and recurses toUnion{}for non-wrapped arrays whosesimilaris overloaded by NDTensors.Defining the missing fallback as overloads on
TypeParameterAccessors.similartype(::Type{<:AbstractArray}, ...)would be type piracy. Instead, this PR introduces an NDTensors-owned helperrecursive_similartype(inabstractarray/similar.jl) that reproduces the trait-based wrapper-unwrap dispatch the vendored copy provided. The helper is used at the small number of call sites that pass a non-NDTensors-ownedAbstractArraytype tosimilartype: NDTensors'ssimilar(::Type{<:AbstractArray}, ...)internals,tensorstorage/similar.jl'ssimilartypeoverloads (recursing intodatatype(...)), anddense/dense.jl'spromote_ruleforDense. NDTensors-owned types (Tensor,TensorStorage) andAbstractMappedArray(inNDTensorsMappedArraysExt) get explicitrecursive_similartypedelegation overloads, since theirparenttypeclassifies them as wrapped underIsWrappedArrayand the trait-based fallback would otherwise strip them down to a plainArray— that incorrect unwrap was the root cause of the_fill!!(::EmptyTensor, ...)materialization breakage hit during iteration.A previously missing 3-arg
similartype(::Type{<:Tensor}, ::Type, ::Tuple)is also added (composing the 2-arg eltype and dims forms) — this was the shape vendored TPA provided generically.Verified locally:
Pkg.test("NDTensors")passes with 2845 passing, 0 failing, 4 broken (pre-existing unrelated JLArray@test_brokenitems inExpose/test/runtests.jl). TheNDTensorsMappedArraysExttest passes 7/7. The 2-thread ITensors run that originally surfaced this passes 18590, with 40 pre-existing broken.Patch bump: NDTensors 0.4.26 -> 0.4.27.