Skip to content

Conversation

@jClugstor
Copy link
Member

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the SciML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

We want to move the solve dispatches for Nonlinear problems to NonlinearSolveBase from DiffEqBase. In order to do that we need to move some of the things from DiffEqBase to SciMLBase so that NonlinearSolveBase can use them.

@jClugstor jClugstor marked this pull request as ready for review August 13, 2025 23:34
@jClugstor
Copy link
Member Author

@ChrisRackauckas this moves the general utility functions used for the solve dispatches to SciMLBase, the extensions needed for those, and most of the errors that were in DiffEqBase before.

I have a branch of DiffEqBase that imports and uses these from SciMLBase, which also means that for example DiffEqBase.anyeltypedual will still work even though it's defined in SciMLBase. I think that should make this non-breaking and allow us to gradually move to using SciMLBase instead of DiffEqBase for these.

@ChrisRackauckas
Copy link
Member

What's the load time impact of this?

@jClugstor
Copy link
Member Author

I did

@time using SciMLBase

Current master: 0.367625 seconds (470.92 k allocations: 41.653 MiB, 2.65% gc time, 12.92% compilation time)

This PR: 0.364500 seconds (385.76 k allocations: 37.483 MiB, 2.90% gc time)

I also looked at loading OrdinaryDiffEq:

@time using OrdinaryDiffEq

Current master: 5.729929 seconds (12.34 M allocations: 696.356 MiB, 2.83% gc time, 11.45% compilation time: 4% of which was recompilation)

This PR: 5.783470 seconds (12.33 M allocations: 695.451 MiB, 3.04% gc time, 11.07% compilation time: 4% of which was recompilation)

Any better way to see how this affects load times?

@ChrisRackauckas ChrisRackauckas merged commit 673c780 into SciML:master Aug 15, 2025
49 of 63 checks passed
RuntimeGeneratedFunctions = "7e49a35a-f44a-4d26-94aa-eba1b4ca6b47"
SciMLOperators = "c0aeaf25-5076-4817-a8d5-81caf7dfa961"
SciMLStructures = "53ae85a6-f571-4167-b2af-e1d143709226"
Static = "aedffcd0-7271-4cad-89d0-dc628f76c6d3"
Copy link
Member

Choose a reason for hiding this comment

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

oh shit, that shouldn't be there... why is static needed? Just noticed this too late. This should not be in SciMLBase!

Copy link
Member Author

Choose a reason for hiding this comment

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

reduce_tup is used for the Dual type checking, anyeltypedual.

Copy link
Member

Choose a reason for hiding this comment

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

can that be isolated from the rest of Static?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think so, I don't see anything in the definition that uses anything besides Base functions. Should I just copy paste this?

@generated function reduce_tup(f::F, inds::Tuple{Vararg{Any, N}}) where {F, N}
    q = Expr(:block, Expr(:meta, :inline, :propagate_inbounds))
    if N == 1
        push!(q.args, :(inds[1]))
        return q
    end
    syms = Vector{Symbol}(undef, N)
    i = 0
    for n in 1:N
        syms[n] = iₙ = Symbol(:i_, (i += 1))
        push!(q.args, Expr(:(=), iₙ, Expr(:ref, :inds, n)))
    end
    W = 1 << (8sizeof(N) - 2 - leading_zeros(N))
    while W > 0
        _N = length(syms)
        for _ in (2W):W:_N
            for w in 1:W
                new_sym = Symbol(:i_, (i += 1))
                push!(q.args, Expr(:(=), new_sym, Expr(:call, :f, syms[w], syms[w + W])))
                syms[w] = new_sym
            end
            deleteat!(syms, (1 + W):(2W))
        end
        W >>>= 1
    end
    q
end

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.

2 participants