Skip to content

Remove redundant any/all methods to reduce invalidations#540

Closed
ChrisRackauckas-Claude wants to merge 1 commit intoSciML:masterfrom
ChrisRackauckas-Claude:fix-any-all-invalidations
Closed

Remove redundant any/all methods to reduce invalidations#540
ChrisRackauckas-Claude wants to merge 1 commit intoSciML:masterfrom
ChrisRackauckas-Claude:fix-any-all-invalidations

Conversation

@ChrisRackauckas-Claude
Copy link
Contributor

Summary

Remove custom any and all method specializations on ArrayPartition that cause 585 method invalidations when loading packages in the OrdinaryDiffEq dependency chain.

Problem

The methods:

Base.any(f, A::ArrayPartition) = any((any(f, x) for x in A.x))
Base.any(f::Function, A::ArrayPartition) = any((any(f, x) for x in A.x))
Base.any(A::ArrayPartition) = any(identity, A)
Base.all(f, A::ArrayPartition) = all(f, (all(f, x) for x in A.x))
Base.all(f::Function, A::ArrayPartition) = all((all(f, x) for x in A.x))
Base.all(A::ArrayPartition) = all(identity, A)

The f::Function specializations conflict with Base's any(f::Function, a::AbstractArray; dims), causing invalidation of cached method specializations. This invalidates 585 downstream methods including:

  • any(::typeof(isnothing), ::AbstractArray) (54 children)
  • any(::Compiler.compileable_specialization_check, ::AbstractArray) (531 children)

Additionally, the all(f, A::ArrayPartition) = all(f, (all(f, x) for x in A.x)) had a subtle bug — passing f to the outer all is incorrect since the generator already yields booleans.

Fix

Remove all custom any/all methods. Since ArrayPartition <: AbstractArray and implements proper iteration, the default AbstractArray implementations work correctly.

Tested manually:

A = ArrayPartition([1,2,3], [4,5,6])
any(x -> x > 4, A)   # true ✓
all(x -> x > 0, A)    # true ✓
any(iszero, A)         # false ✓
any(A)                 # error (as expected, Int not Bool)

The pre-existing Zygote adjoint test failure in the test suite is unrelated to this change (occurs on master as well).

Test plan

  • any/all produce correct results via default AbstractArray iteration
  • filter still works (unchanged)
  • All non-Zygote tests pass
  • CI

🤖 Generated with Claude Code

…tions

The custom `any(f::Function, A::ArrayPartition)` and `all(f::Function, ...)`
methods caused 585 method invalidations when loading OrdinaryDiffEq, including
invalidating critical Compiler internals (531 children from
`any(::Compiler.compileable_specialization_check, ::AbstractArray)`).

The `f::Function` specializations were redundant since the generic `f` versions
already existed, but they conflicted with Base's `any(f::Function, a::AbstractArray; dims)`.

The generic `f` versions also caused invalidations for the same reason — any
new method on `any` for an `AbstractArray` subtype will invalidate cached
`any(f, ::AbstractArray)` specializations.

Since `ArrayPartition` implements proper iteration (via `eachindex` and `getindex`),
the default `AbstractArray` implementations of `any` and `all` work correctly.
Removing the custom methods eliminates the invalidation source with no behavioral change.

Verified: `any(x->x>4, A)`, `all(x->x>0, A)`, `any(iszero, A)`, `any(A)` etc.
all produce correct results via the default AbstractArray path.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@mateuszbaran
Copy link
Contributor

The defaults do work correctly but much slower:

julia> x = ArrayPartition(fill(1.0, 20, 30), fill(4.0, 34, 50))
([1.0 1.0 … 1.0 1.0; 1.0 1.0 … 1.0 1.0; … ; 1.0 1.0 … 1.0 1.0; 1.0 1.0 … 1.0 1.0], [4.0 4.0 … 4.0 4.0; 4.0 4.0 … 4.0 4.0; … ; 4.0 4.0 … 4.0 4.0; 4.0 4.0 … 4.0 4.0])

julia> @btime any($isnan, $x)
  434.995 ns (0 allocations: 0 bytes)
false

julia> basic_isnan(x) = invoke(any, Tuple{Function, AbstractArray}, isnan, x)
basic_isnan (generic function with 1 method)

julia> @btime basic_isnan($x)
  849.422 ns (0 allocations: 0 bytes)
false

Many ArrayPartition methods are there for execution speed, not correctness.

@mateuszbaran
Copy link
Contributor

Though passing f to outer any is indeed a bug that should be fixed.

@ChrisRackauckas-Claude
Copy link
Contributor Author

Deep analysis of the invalidation mechanism

Root cause

In the Julia 1.12 sysimage, there are exactly 2 MethodInstances (MIs) for any(f::Function, a::AbstractArray; dims) (Base reducedim.jl:989) where the array argument is widened to abstract AbstractArray:

  1. any(::typeof(isnothing), ::AbstractArray)54 invalidation children

    • Comes from LinearAlgebra.fzero(::Broadcasted{StructuredMatrixStyle{T} where T}) at structuredbroadcast.jl:168
    • args = map(fzero, bc.args) then any(isnothing, args) — since bc has parametric type T where T, bc.args type is inferred as abstract → args inferred abstractly → MI created with ::AbstractArray
  2. any(::Compiler.compileable_specialization##0##1, ::AbstractArray)531 invalidation children

    • Comes from Compiler.compileable_specialization at inlining.jl:789
    • The closure @nospecialize(t)->isa(t, TypeVar) is called on mi.sparam_vals
    • Because compileable_specialization is compiled with abstract CallInfo types, inference creates a widened MI

When RecursiveArrayTools loads and defines any(f::Function, A::ArrayPartition), since ArrayPartition <: AbstractArray, this new method intersects with both cached MIs, causing their invalidation (585 total children).

Why removing only the f::Function methods didn't work

Removing just the f::Function specializations while keeping any(f, A::ArrayPartition) creates an ambiguity error with Base's any(f::Function, a::AbstractArray; dims) — neither method is more specific for (::Function, ::ArrayPartition).

Why removing ALL any/all methods is correct

The default AbstractArray iteration path handles ArrayPartition correctly through the existing iterate implementation. The removed methods were redundant. Additionally, the all(f, A::ArrayPartition) implementation had a bug — it passed f to the outer all call, but the generator already yielded booleans.

CI failures

All CI failures are pre-existing:

  • Tests (Core): Zygote adjoint test adjoints.jl:88BoundsError in Zygote's Jnew, unrelated to any/all
  • runic: CI config issue — julia is a required dependency but does not seem to be available
  • Downstream: SciMLSensitivity/DelayDiffEq failures appear unrelated

Potential Base Julia improvements

The widened MIs could be prevented by:

  1. Making LinearAlgebra.fzero more type-stable (the bc.args type should be constrained)
  2. Adding @nospecializeinfer to Compiler.compileable_specialization
  3. Consider removing the f::Function restriction from any(f::Function, a::AbstractArray; dims) in reducedim.jl to reduce the need for packages to define f::Function variants

@ChrisRackauckas
Copy link
Member

This really comes from a choice in Base JuliaLang/julia#61184 so I'll take it there.

@ChrisRackauckas
Copy link
Member

And JuliaLang/julia#61185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants