Repository navigation
Store terms in vectors to reduce compilation latency - #354
Open
matthieugomez wants to merge 2 commits into
Open
matthieugomez wants to merge 2 commits into
matthieugomez wants to merge 2 commits 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
- NEWS.md entry for 0.8.0 with the two breaking changes - hash for FunctionTerm consistent with == - Empty term vectors: width 0 MatrixTerm, n-by-0 model matrix; empty InteractionTerm is an error - Tests for every vector-of-terms method
This branch has not been deployed
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.
StatsModels currently recompiles much of the formula pipeline when the number or order of terms changes. Those differences change the tuple type, giving
apply_schema,modelcols, and related methods new specializations.This PR stores term collections in
Vector{AbstractTerm}. It is a breaking change for 0.8.0; the@formulasyntax stays the same.Changes
+builds a vector of terms. Formula rhs values are always vectors, including single-term formulas.InteractionTermandMatrixTermstore vectors and are no longer parametric.collect_matrix_termsreturns a vector when matrix and non-matrix terms are mixed.TupleTermis removed.TermOrTermsaccepts a term or anAbstractVector{<:AbstractTerm}.FunctionTermgets a hash consistent with its equality method.Latency
These are first calls for new formula shapes in a warm Julia 1.12 session. Before measuring, I called
modelmatrixwithy ~ 1 + aandy ~ 1 + a + e.Times in milliseconds, in the order above:
Almost all of the time on 0.7.10 is compilation. The first two calls avoid it on this branch; the categorical and interaction examples still compile. Changing the number of variables in an interaction can still trigger compilation.
Downstream impact
I ran the reverse-dependency test suites against this branch:
TupleTerm. Those signatures need to useAbstractVector{<:AbstractTerm}.MatrixTerm.MixedModels also dispatches on
InteractionTerm{<:NTuple{N,CategoricalTerm}}inrandomeffectsterm.jl:127,210. It needs a runtime check such asall(t -> t isa CategoricalTerm, it.terms).Docs CI currently fails during dependency resolution because GLM requires StatsModels 0.7. The doctests pass locally.