Skip to content

Static fixes #45

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Static fixes #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5257c76
Unify static_<first/step/last> with `Static`
Tokazama Oct 19, 2025
e0cca5c
Unify static_<first/step/last> with `Static`
Tokazama Oct 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
name = "StaticArrayInterface"
uuid = "0d7ed370-da01-4f52-bd93-41d350b8b718"
version = "1.8.0"
version = "1.8.1"

[deps]
ArrayInterface = "4fba245c-0d91-5ea0-9b3e-6abc04ee57a9"
Expand Down
75 changes: 40 additions & 35 deletions src/StaticArrayInterface.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,34 +19,35 @@ using PrecompileTools
@recompile_invalidations begin
using ArrayInterface
import ArrayInterface: allowed_getindex, allowed_setindex!, aos_to_soa, buffer,
parent_type, fast_matrix_colors, findstructralnz,
has_sparsestruct,
issingular, isstructured, matrix_colors, restructure,
lu_instance,
safevec, zeromatrix, undefmatrix, ColoringAlgorithm,
fast_scalar_indexing, parameterless_type,
is_forwarding_wrapper,
map_tuple_type, flatten_tuples, GetIndex, SetIndex!,
defines_strides, ndims_index, ndims_shape,
stride_preserving_index
parent_type, fast_matrix_colors, findstructralnz,
has_sparsestruct,
issingular, isstructured, matrix_colors, restructure,
lu_instance,
safevec, zeromatrix, undefmatrix, ColoringAlgorithm,
fast_scalar_indexing, parameterless_type,
is_forwarding_wrapper,
map_tuple_type, flatten_tuples, GetIndex, SetIndex!,
defines_strides, ndims_index, ndims_shape,
stride_preserving_index

# ArrayIndex subtypes and methods
import ArrayInterface: ArrayIndex, MatrixIndex, VectorIndex, BidiagonalIndex,
TridiagonalIndex
TridiagonalIndex
# managing immutables
import ArrayInterface: ismutable, can_change_size, can_setindex
# constants
import ArrayInterface: MatAdjTrans, VecAdjTrans, UpTri, LoTri
# device pieces
import ArrayInterface: AbstractDevice, AbstractCPU, CPUPointer, CPUTuple, CheckParent,
CPUIndex, GPU, can_avx, device
CPUIndex, GPU, can_avx, device

using Static
using Static: Zero, One, nstatic, eq, ne, gt, ge, lt, le, eachop, eachop_tuple,
permute, invariant_permutation, field_type, reduce_tup, find_first_eq,
OptionallyStaticUnitRange, OptionallyStaticStepRange, OptionallyStaticRange,
IntType,
SOneTo, SUnitRange
permute, invariant_permutation, field_type, reduce_tup, find_first_eq,
OptionallyStaticUnitRange, OptionallyStaticStepRange,
OptionallyStaticRange,
IntType,
SOneTo, SUnitRange, static_first, static_step, static_last

using IfElse

Expand Down Expand Up @@ -77,26 +78,26 @@ known_offset1, known_offsets, known_size, known_step, known_strides
Subtype of `ArrayIndex` that transforms and index using stride layout information
derived from `x`.
"""
struct StrideIndex{N,R,C,S,O} <: ArrayIndex{N}
struct StrideIndex{N, R, C, S, O} <: ArrayIndex{N}
strides::S
offsets::O
@inline function StrideIndex{N,R,C}(s::S, o::O) where {N,R,C,S,O}
return new{N,R::NTuple{N,Int},C,S,O}(s, o)
@inline function StrideIndex{N, R, C}(s::S, o::O) where {N, R, C, S, O}
return new{N, R::NTuple{N, Int}, C, S, O}(s, o)
end
end

"""
LazyAxis{N}(parent::AbstractArray)
A lazy representation of `axes(parent, N)`.
"""
struct LazyAxis{N,P} <: AbstractUnitRange{Int}
struct LazyAxis{N, P} <: AbstractUnitRange{Int}
parent::P

function LazyAxis{N}(parent::P) where {N,P}
N > 0 && return new{N::Int,P}(parent)
function LazyAxis{N}(parent::P) where {N, P}
N > 0 && return new{N::Int, P}(parent)
throw_dim_error(parent, N)
end
@inline LazyAxis{:}(parent::P) where {P} = new{ifelse(ndims(P) === 1, 1, :),P}(parent)
@inline LazyAxis{:}(parent::P) where {P} = new{ifelse(ndims(P) === 1, 1, :), P}(parent)
end

function throw_dim_error(@nospecialize(x), @nospecialize(dim))
Expand All @@ -111,9 +112,10 @@ An abstract trait that is used to determine how axes are combined when calling `
"""
abstract type BroadcastAxis end

@assume_effects :total function _find_first_true(isi::Tuple{Vararg{Union{Bool, Static.StaticBool}, N}}) where {N}
@assume_effects :total function _find_first_true(isi::Tuple{Vararg{
Union{Bool, Static.StaticBool}, N}}) where {N}
for i in 1:N
x = getfield(isi, i)
x = getfield(isi, i)
if (x isa Bool && x === true) || x isa Static.True
return i
end
Expand Down Expand Up @@ -235,7 +237,8 @@ struct IndicesInfo{Np, pdims, cdims, Nc}
IndicesInfo(x::SubArray) = IndicesInfo{ndims(parent(x))}(typeof(x.indices))
end

@inline function _map_splats(nsplat::Int, splat_index::Int, dims::Tuple{Vararg{Union{Int,StaticInt}}})
@inline function _map_splats(nsplat::Int, splat_index::Int, dims::Tuple{Vararg{Union{
Int, StaticInt}}})
ntuple(length(dims)) do i
i === splat_index ? (nsplat * getfield(dims, i)) : getfield(dims, i)
end
Expand Down Expand Up @@ -323,9 +326,9 @@ end
return setindex!(A, val; kwargs...)
end

@inline static_first(x) = Static.maybe_static(known_first, first, x)
@inline static_last(x) = Static.maybe_static(known_last, last, x)
@inline static_step(x) = Static.maybe_static(known_step, step, x)
@inline Static.static_first(x::AbstractArray) = Static.maybe_static(known_first, first, x)
@inline Static.static_last(x::AbstractArray) = Static.maybe_static(known_last, last, x)
@inline Static.static_step(x::AbstractArray) = Static.maybe_static(known_step, step, x)
Comment on lines +329 to +331
Copy link

@devmotion devmotion Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is type piracy - neither method nor argument is owned by StaticArrayInterface. I think this is not a good fix.

Copy link
Member

@ChrisRackauckas ChrisRackauckas Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Why does it exist in Array interface at all? It's a static thing?

Copy link
Member Author

@Tokazama Tokazama Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is type piracy - neither method nor argument is owned by StaticArrayInterface. I think this is not a good fix.

True but the alternative may be more involved.

Good point. Why does it exist in Array interface at all? It's a static thing?

The alternative solution is to move these known_... methods to Static. I'm fine with doing this but it might take some extra finesse. I'll try moving them to Static.

Copy link

@devmotion devmotion Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's possible to remove these definitions here completely and just use the Static versions?

Copy link
Member Author

@Tokazama Tokazama Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After refamiliarizing myself with this code I think we have a couple options:

  1. Accept that StaticArrayInterface is the sole package that gets to do this type piracy because it is specifically for arrays
  2. Move all the known_<...> methods to Static and have Static depend on ArrayInterface. We originally wanted Static to be a fairly light dependency, so moving all static related things there wasn't the go to. However, that may not be realistic since Base Julia would ultimately need to change first.
  3. Move known_<...> methods to ArrayInterface since those methods don't require StaticInt. Then define static_<...> methods in Static. Basically, same approach as 2 but different execution. This would take a lot more work to disentangle things but could prepare us to eventually not be so dependent on Static. I'm not sure how we can completely remove it as a dependency, but if invalidations aren't every going to get fixed I'm not sure we have a choice.

Copy link
Member

@ChrisRackauckas ChrisRackauckas Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the packages using this?

Copy link

@devmotion devmotion Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When searching for static_first on JuliaHub, it seems only Static.jl uses Static.static_first but all other packages use StaticArrayInterface.static_first: https://juliahub.com/ui/Search?q=static_first&type=code

Maybe the Static methods should just not be exported?

Copy link
Member Author

@Tokazama Tokazama Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When searching for static_first on JuliaHub, it seems only Static.jl uses Static.static_first but all other packages use StaticArrayInterface.static_first: https://juliahub.com/ui/Search?q=static_first&type=code

Maybe the Static methods should just not be exported?

I'd be fine with that or do we want to push for static stuff to be in Static.jl?

Copy link
Member

@ChrisRackauckas ChrisRackauckas Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either export or move to Static.jl, but I don't think anyone actually cares or uses this.


@inline function _to_cartesian(a, i::IntType)
@inbounds(CartesianIndices(ntuple(dim -> indices(a, dim), Val(ndims(a))))[i])
Expand Down Expand Up @@ -434,7 +437,7 @@ Base.@propagate_inbounds function deleteat(collection::AbstractVector, index)
return unsafe_deleteat(collection, index)
end
Base.@propagate_inbounds function deleteat(collection::Tuple{Vararg{Any, N}},
index) where {N}
index) where {N}
@boundscheck if !checkindex(Bool, StaticInt{1}():StaticInt{N}(), index)
throw(BoundsError(collection, index))
end
Expand Down Expand Up @@ -504,11 +507,13 @@ include("broadcast.jl")
# Putting some things in `setup` can reduce the size of the
# precompile file and potentially make loading faster.
arrays = [rand(4), Base.oneto(5)]
@compile_workload begin for x in arrays
known_first(x)
known_step(x)
known_last(x)
end end
@compile_workload begin
for x in arrays
known_first(x)
known_step(x)
known_last(x)
end
end
end

end
Loading