Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IndexKeyMap #192

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

IndexKeyMap #192

wants to merge 16 commits into from

Conversation

chriselrod
Copy link
Contributor

@chriselrod chriselrod commented Jul 13, 2023

This PR includes #191.

With the PR:

julia> @benchmark loglikelihood($model, $rp)
BenchmarkTools.Trial: 9095 samples with 1 evaluation.
 Range (min  max):   84.046 μs   13.707 ms  ┊ GC (min  max):  0.00%  97.32%
 Time  (median):      91.796 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   107.322 μs ± 315.849 μs  ┊ GC (mean ± σ):  10.79% ±  3.76%

    ▂▄██▄▁                                                       
  ▁▄██████▇▅▄▄▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  84 μs            Histogram: frequency by time          167 μs <

 Memory estimate: 75.25 KiB, allocs estimate: 486.

Master is around 400us, or maybe around 200us with the latest Unityper.

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Merging #192 (d7f6e71) into main (38a8eaf) will decrease coverage by 2.66%.
The diff coverage is 80.29%.

@@            Coverage Diff             @@
##             main     #192      +/-   ##
==========================================
- Coverage   81.77%   79.11%   -2.66%     
==========================================
  Files           7        8       +1     
  Lines         439      613     +174     
==========================================
+ Hits          359      485     +126     
- Misses         80      128      +48     
Impacted Files Coverage Δ
src/EasyModelAnalysis.jl 100.00% <ø> (ø)
src/datafit.jl 76.51% <76.10%> (+32.07%) ⬆️
src/ensemble.jl 90.47% <86.66%> (-9.53%) ⬇️
src/keyindexmap.jl 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@chriselrod
Copy link
Contributor Author

chriselrod commented Jul 19, 2023

We're allocating over 10_000 times as much memory as needed for the returned objects:

julia> sol2 = @time solve(fit_enprob; saveat = t_forecast);
717.090530 seconds (5.89 G allocations: 865.902 GiB, 27.12% gc time)

so performance is dreadful.
Only 27% GC time is a surprisingly good showing from the GC (on Julia 1.9).

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