bankofcanada / ModelBaseEcon.jl

BSD 3-Clause "New" or "Revised" License
26 stars 5 forks source link

Avoid creating a new generator to create `LittleDict` for every equation #32

Closed KristofferC closed 1 year ago

KristofferC commented 1 year ago

This takes the time to precompile the FRB_US model from 52s to 38s.

This also takes the load time of the precompiled FRB_US model from 5.5s to 0.16s which is a bigger change than I expected.

codecov-commenter commented 1 year ago

Codecov Report

Merging #32 (ffb4218) into master (32835b1) will not change coverage. The diff coverage is n/a.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master      #32   +/-   ##
=======================================
  Coverage   94.20%   94.20%           
=======================================
  Files          15       15           
  Lines        1675     1675           
=======================================
  Hits         1578     1578           
  Misses         97       97           
Impacted Files Coverage Δ
src/evaluation.jl 80.33% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

bbejanov commented 1 year ago

Wow, I just found out last week how costly generators are. Thanks for fixing this @KristofferC! https://discourse.julialang.org/t/slow-dict-comprehension/94797

KristofferC commented 1 year ago

Heh, in one of the replies in https://discourse.julialang.org/t/slow-dict-comprehension/94797/5?u=kristoffer.carlsson

So I would say you’re fine using a comprehension in practice. It’s unlikely you’ll have hundreds of these in your top-level script?

image