JuliaDynamics / Agents.jl

Agent-based modeling framework in Julia
https://juliadynamics.github.io/Agents.jl/stable/
MIT License
717 stars 115 forks source link

Improve speed of random_agent #897

Closed Tortar closed 10 months ago

Tortar commented 10 months ago

It was actually very slow before, consider this silly model I was using to benchmark a different thing:

using Agents, Random, BenchmarkTools

function init_model()
    model = StandardABM(GridAgent{2}, GridSpace((50, 50)), rng=Xoshiro(42))
    for _ in 1:1000
        add_agent!(model)
    end
    return model
end

function model_step!(model)
    add_agent!(model)
    remove_agent!(random_agent(model), model)
end

model = init_model()
@benchmark step!(model, dummystep, model_step!, 10^5)

before:

julia> @benchmark step!(model, dummystep, model_step!, 10^5)
BenchmarkTools.Trial: 27 samples with 1 evaluation.
 Range (min … max):  183.797 ms … 194.004 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     187.722 ms               ┊ GC (median):    0.00%
 Time  (mean ± σ):   188.308 ms ±   2.707 ms  ┊ GC (mean ± σ):  0.13% ± 0.37%

                 ▃ █     ▃
  ▇▁▁▇▁▁▁▁▁▇▁▇▁▁▇█▁█▇▁▇▇▁█▁▇▁▁▁▁▇▇▁▇▇▇▁▇▁▁▁▁▇▁▁▁▁▁▁▁▁▇▁▁▁▁▇▇▁▁▇ ▁
  184 ms           Histogram: frequency by time          194 ms <

 Memory estimate: 5.19 MiB, allocs estimate: 100169.

after:

julia> @benchmark step!(model, dummystep, model_step!, 10^5)
BenchmarkTools.Trial: 306 samples with 1 evaluation.
 Range (min … max):  15.251 ms … 26.520 ms  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     15.822 ms              ┊ GC (median):    0.00%
 Time  (mean ± σ):   16.360 ms ±  1.440 ms  ┊ GC (mean ± σ):  1.50% ± 3.67%

    ▄█▆▁
  ▃▅████▇▆▃▃▄▅▄▄▃▃▁▃▃▃▃▄▃▃▃▃▃▂▂▂▃▂▃▁▁▂▁▁▁▁▁▃▂▁▁▁▁▂▁▂▁▁▁▁▁▂▂▁▂ ▃
  15.3 ms         Histogram: frequency by time        21.7 ms <

 Memory estimate: 5.19 MiB, allocs estimate: 100162.

A micro-benchmark tells that now random_agent for a StandardABM is 40x faster!

I exported a new function random_id in the process, which is in line to the other random functions which export also the id version

Tortar commented 10 months ago

I have to say that this means that probably there is something to improve in Base, since that the previous method is so slow sounds strange

codecov-commenter commented 10 months ago

Codecov Report

Merging #897 (737312d) into main (a71cc12) will increase coverage by 12.09%. The diff coverage is 60.00%.

:exclamation: Current head 737312d differs from pull request most recent head 90a22d6. Consider uploading reports for the commit 90a22d6 to get more accurate results

@@             Coverage Diff             @@
##             main     #897       +/-   ##
===========================================
+ Coverage   80.27%   92.37%   +12.09%     
===========================================
  Files          44       32       -12     
  Lines        3027     2334      -693     
===========================================
- Hits         2430     2156      -274     
+ Misses        597      178      -419     
Files Coverage Δ
src/core/model_abstract.jl 89.06% <66.66%> (-0.03%) :arrow_down:
src/core/model_concrete.jl 88.23% <50.00%> (-1.18%) :arrow_down:

... and 18 files with indirect coverage changes

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

Tortar commented 10 months ago

Actually it was a O(N) operation while this one is not, this means that the improvement in speed is arbitrary large!

Tortar commented 10 months ago

I think it should be https://github.com/JuliaLang/julia/issues/51605 , let's see :-)