search

FixedEffects / FixedEffectModels.jl

Fast Estimation of Linear Models with IV and High Dimensional Categorical Variables
Other
227 stars 46 forks source link

Rebased version of reverted #109 #205

Open greimel opened 2 years ago

greimel commented 2 years ago

My initial PR #109 was merged and then reverted

out of concern with memory allocation

by @eloualiche. I benchmarked this PR vs master (using the middle regression model in benchmarks/benchmarks.jl

# PR
julia> @btime reg($df, @formula(y ~ x1 + x2 + fe(id1) + fe(id2)))
  2.842 s (38190 allocations: 97.22 MiB)

# master
@btime reg($df, @formula(y ~ x1 + x2 + fe(id1) + fe(id2)))
  2.713 s (35578 allocations: 90.64 MiB)

Is this difference significant enough to prevent this feature?

greimel commented 2 years ago

I've gotten rid of the excess allocations.

codecov-commenter commented 2 years ago

Codecov Report

Merging #205 (672dd78) into master (2662649) will decrease coverage by 0.63%. The diff coverage is 89.36%.

@@            Coverage Diff             @@
##           master     #205      +/-   ##
==========================================
- Coverage   96.08%   95.45%   -0.64%     
==========================================
  Files           8        8              
  Lines         588      638      +50     
==========================================
+ Hits          565      609      +44     
- Misses         23       29       +6
Impacted Files Coverage Δ
src/FixedEffectModels.jl 100.00% <ø> (ø)
src/FixedEffectModel.jl 92.99% <76.19%> (-0.56%) :arrow_down:
src/partial_out.jl 91.66% <84.00%> (-3.86%) :arrow_down:
src/fit.jl 96.31% <97.91%> (+0.06%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4887c70...672dd78. Read the comment docs.

nilshg commented 12 months ago

Can this be closed as missing is now supported?