JuliaDynamics / Agents.jl

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

Further clarify mandatory API `ABM` #900

Closed Datseris closed 10 months ago

Datseris commented 10 months ago

In preparation for the new Gillespie model, this PR closes #899 .

I've also removed the agent type as a type parameter of the abstract AgentBasedModel, in foresight of future multi-agent models not being parameterized by a Union type. This change took me 2 hours to do.

Datseris commented 10 months ago

This is very frustrating. The tests of the IO are failing everywhere. This is really annoying that this part of the code is so hardcoded and doesn't use API functions.

Datseris commented 10 months ago

Okay I hope to get some help here. We have only two failures. One is for sure with the change @Tortar did with the ID iterator. I get:

sample!: Error During Test at C:\Users\gd419\.julia\dev\Agents\test\randomness_tests.jl:19
  Got exception outside of a @test
  MethodError: no method matching keys(::Base.ValueIterator{Dict{Int64, Agent1}})

  Closest candidates are:
    keys(::Union{Tables.AbstractColumns, Tables.AbstractRow})
     @ Tables C:\Users\gd419\.julia\packages\Tables\u1X7W\src\Tables.jl:189
    keys(::DataStructures.Trie)
     @ DataStructures C:\Users\gd419\.julia\packages\DataStructures\MKv4P\src\trie.jl:82
    keys(::DataStructures.Trie, ::AbstractString)
     @ DataStructures C:\Users\gd419\.julia\packages\DataStructures\MKv4P\src\trie.jl:82
    ...

  Stacktrace:
    [1] (::var"#159#172"{Base.ValueIterator{Dict{Int64, Agent1}}, StandardABM{GridSpace{2,
typeof(dummystep), typeof(dummystep), typeof(Agents.Schedulers.fastest), Nothing, StableRNf::Symbol)
      @ Main .\none:0
    [2] iterate
Agents.Schedulers.fastest), Nothing, StableRNGs.LehmerRNG}}}})
      @ Base .\array.jl:707
    [8] macro expansion
      @ C:\Users\gd419\.julia\dev\Agents\test\randomness_tests.jl:29 [inlined]
    [9] macro expansion
      @ C:\Users\gd419\.julia\juliaup\julia-1.9.1+0.x64.w64.mingw32\share\julia\stdlib\v1.9\Test\src\Test.jl:1498 [inlined]
   [10] top-level scope
      @ C:\Users\gd419\.julia\dev\Agents\test\randomness_tests.jl:20
   [11] include(fname::String)
      @ Base.MainInclude .\client.jl:478
   [12] macro expansion
      @ C:\Users\gd419\.julia\dev\Agents\test\runtests.jl:153 [inlined]
   [13] macro expansion
      @ C:\Users\gd419\.julia\juliaup\julia-1.9.1+0.x64.w64.mingw32\share\julia\stdlib\v1.9\Test\src\Test.jl:1498 [inlined]
   [14] top-level scope
      @ C:\Users\gd419\.julia\dev\Agents\test\runtests.jl:151
   [15] include(fname::String)
      @ Base.MainInclude .\client.jl:478
   [16] top-level scope
      @ none:6
   [17] eval
      @ .\boot.jl:370 [inlined]
   [18] exec_options(opts::Base.JLOptions)
      @ Base .\client.jl:280
   [19] _start()
      @ Base .\client.jl:522

The other failure is related with the AgentsIO tests checking the internal nextid field. The tests fail because the generated model has nextid 0. I don't know why but maybe @AayushSabharwal can help:

ContinuousSpace: Test Failed at C:\Users\gd419\.julia\dev\Agents\test\jld2_tests.jl:9
  Expression: (getfield(model, :maxid)).x == (getfield(other, :maxid)).x
   Evaluated: 0 == 10

Stacktrace:
 [1] macro expansion
   @ C:\Users\gd419\.julia\juliaup\julia-1.9.1+0.x64.w64.mingw32\share\julia\stdlib\v1.9\Test\src\Test.jl:478 [inlined]
 [2] (::var"#test_model_data#572")(model::UnremovableABM{ContinuousSpace{2, true, Float64, 
typeof(Agents.no_vel_update)}, Bird, typeof(flocking_model_agent_step!), typeof(dummystep), Agents.Schedulers.Randomly, Nothing, StableRNGs.LehmerRNG}, other::StandardABM{ContinuousSpace{2, true, Float64, typeof(Agents.no_vel_update)}, Bird, typeof(dummystep), typeof(dummystep), Agents.Schedulers.Randomly, Nothing, StableRNGs.LehmerRNG})
   @ Main C:\Users\gd419\.julia\dev\Agents\test\jld2_tests.jl:9
GridSpace: Test Failed at C:\Users\gd419\.julia\dev\Agents\test\jld2_tests.jl:9
  Expression: (getfield(model, :maxid)).x == (getfield(other, :maxid)).x       
   Evaluated: 0 == 30

Stacktrace:
 [1] macro expansion
   @ C:\Users\gd419\.julia\juliaup\julia-1.9.1+0.x64.w64.mingw32\share\julia\stdlib\v1.9\Test\src\Test.jl:478 [inlined]
 [2] (::var"#test_model_data#572")(model::UnremovableABM{GridSpace{2, false}, SchellingAgent, typeof(schelling_model_agent_step!), typeof(dummystep), Agents.Schedulers.Randomly, Dict{Symbol, Int64}, StableRNGs.LehmerRNG}, other::StandardABM{GridSpace{2, false}, SchellingAgent, typeof(dummystep), typeof(dummystep), Agents.Schedulers.Randomly, Dict{Symbol, Int64}, StableRNGs.LehmerRNG})
   @ Main C:\Users\gd419\.julia\dev\Agents\test\jld2_tests.jl:9

Other than these 2 failures, this PR is good to go.

Tortar commented 10 months ago

will take a look at the failures and review the PR soon!

AayushSabharwal commented 10 months ago

The second test failure (:maxid not matching) is because loading a model from a file always creates a StandardABM, which increments maxid as agents are added. However, the model that was serialized initially is an UnremovableABM, which does not use maxid

Datseris commented 10 months ago

@AayushSabharwal what's the fix?

AayushSabharwal commented 10 months ago

I'll have to experiment with it and see. Shouldn't be too hard

Tortar commented 10 months ago

I'm solving both of the problems @AayushSabharwal, thanks for bisecting the cause (saying this so that we don't do the double of the work)

codecov-commenter commented 10 months ago

Codecov Report

Merging #900 (0f97544) into main (fd62258) will increase coverage by 11.84%. The diff coverage is 87.64%.

:exclamation: Current head 0f97544 differs from pull request most recent head 714853c. Consider uploading reports for the commit 714853c to get more accurate results

@@             Coverage Diff             @@
##             main     #900       +/-   ##
===========================================
+ Coverage   80.21%   92.06%   +11.84%     
===========================================
  Files          44       33       -11     
  Lines        3028     2331      -697     
===========================================
- Hits         2429     2146      -283     
+ Misses        599      185      -414     
Files Coverage Δ
src/Agents.jl 100.00% <ø> (ø)
src/core/higher_order_iteration.jl 83.33% <ø> (ø)
src/core/model_single_container.jl 90.00% <100.00%> (ø)
src/simulations/collect.jl 96.56% <100.00%> (+0.02%) :arrow_up:
src/simulations/sample.jl 100.00% <100.00%> (ø)
src/spaces/continuous.jl 92.34% <100.00%> (+0.04%) :arrow_up:
src/spaces/discrete.jl 98.87% <100.00%> (ø)
src/spaces/graph.jl 76.19% <100.00%> (ø)
src/spaces/grid_multi.jl 83.33% <100.00%> (ø)
src/spaces/grid_single.jl 100.00% <100.00%> (ø)
... and 12 more

... and 12 files with indirect coverage changes

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