Replace term tuples with term vectors to reduce compilation latency - #354
Open
matthieugomez wants to merge 1 commit into
Open
Replace term tuples with term vectors to reduce compilation latency#354matthieugomez wants to merge 1 commit into
matthieugomez wants to merge 1 commit into
Conversation
- Represent term collections as Vector{AbstractTerm}: + returns a vector,
InteractionTerm and MatrixTerm store Vector{AbstractTerm}, and the rhs
built by ~ / @formula is always a vector (a lone term is wrapped)
- Apply schemas by looping over the vector, so methods compile once per
term type instead of once per formula shape
- Define content-based == and hash for FormulaTerm, InteractionTerm, and
MatrixTerm (vector fields break the egal fallback)
- Remove TupleTerm; TermOrTerms is now Union{AbstractTerm, AbstractVector{<:AbstractTerm}}
- Update tests and doctests; bump version to 0.8.0
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.
Fitting a model with a never-seen formula currently costs ~0.1–0.8s of compilation, even when every term type is already precompiled. The reason is that formulas are encoded as tuples, so
apply_schema,modelcols,coefnames, etc. get a fresh specialization for every distinct number and order of terms, and likewise every method taking aMatrixTerm{Ts<:TupleTerm}.This PR represents term collections as
Vector{AbstractTerm}instead of tuples of terms (or a single term when there was only one variable in the formula). This is a breaking change (0.8.0).Changes
+on terms returnsVector{AbstractTerm}.~(and@formula) always makes the rhs a vector, even for a single term (@formula(y ~ x).rhs == [term(:x)]), removing the lone-term special case. The lhs stays a bare term, and afterapply_schemathe rhs still collapses to a bareMatrixTerm, soy, X = modelcols(f, data)is unchanged.InteractionTermandMatrixTermare no longer parametric and store aVector{AbstractTerm}.collect_matrix_termsreturns a vector in the mixed matrix/non-matrix case.apply_schemaover a vector loops term by term, with the same left-to-rightFullRanksemantics as the tuple broadcast.TupleTermis removed;TermOrTermsis nowUnion{AbstractTerm, AbstractVector{<:AbstractTerm}}.==andhashforFormulaTerm,InteractionTerm, andMatrixTerm, since vector fields break the default===fallback. This also fixes formula equality afterapply_schema(a former@test_broken).Latency
Timings below are for the first
modelmatrixcall on a formula shape not seen before, in a warm session on Julia 1.12, after warming up the pipeline withy ~ 1 + aandy ~ 1 + a + e:StatsModels 0.7.10: every new shape recompiles the pipeline (~99.9% compilation time):
This PR:
Downstream impact
I ran the test suites of the main reverse dependencies against this branch.
TupleTermis gone (fix is a one-line signature swap toAbstractVector{<:AbstractTerm}per method): RegressionFormulae, MixedModels, RCall, AnovaBase, HurdleDMR, MixedModelsSerialization.isa MatrixTerm{Tuple{...}}checks), each a few lines: Econometrics, FixedEffectModels, ShiftShares, Metida, SurvivalAnalysis.The only substantive migration is in MixedModels: the
InteractionTerm{<:NTuple{N,CategoricalTerm}}dispatch (randomeffectsterm.jl:127,210) has to become a runtime check (all(t -> t isa CategoricalTerm, it.terms)), since interaction element types no longer live in the type domain.