New model with agent_step! and model_step!

[Tortar]

Fixes https://github.com/JuliaDynamics/Agents.jl/issues/883.

Still need to be applied to all the library examples and tests and we need to deprecate the visualization functions which require to pass the stepping functions from the outside and create new ones. But this works.

Before finishing it, I'd like to ask @Datseris if this is the right fix.

After this PR we can release 6.0!

[codecov-commenter]

Codecov Report

Merging #889 (63b5f0d) into main (ce828f8) will decrease coverage by 0.16%. Report is 3 commits behind head on main. The diff coverage is 89.47%.

:exclamation: Current head 63b5f0d differs from pull request most recent head a95f61d. Consider uploading reports for the commit a95f61d to get more accurate results

@@            Coverage Diff             @@
##             main     #889      +/-   ##
==========================================
- Coverage   92.41%   92.25%   -0.16%     
==========================================
  Files          32       32              
  Lines        2321     2325       +4     
==========================================
  Hits         2145     2145              
- Misses        176      180       +4     

Files	Coverage Δ
src/core/model_abstract.jl	89.09% <100.00%> (-1.68%)	:arrow_down:
src/core/space_interaction_API.jl	93.07% <100.00%> (ø)
src/simulations/collect.jl	96.53% <100.00%> (+0.35%)	:arrow_up:
src/simulations/step.jl	100.00% <100.00%> (ø)
src/spaces/utilities.jl	100.00% <100.00%> (ø)
src/submodules/io/jld2_integration.jl	100.00% <100.00%> (ø)
src/simulations/ensemblerun.jl	91.89% <85.71%> (-8.11%)	:arrow_down:
src/simulations/paramscan.jl	91.89% <75.00%> (-2.40%)	:arrow_down:
src/submodules/schedulers.jl	98.13% <75.00%> (-0.91%)	:arrow_down:
src/core/model_concrete.jl	89.41% <66.66%> (+0.25%)	:arrow_up:

... and 1 file with indirect coverage changes

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

[Datseris]

I will review this after we finalize the discussion in #891

[Datseris]

Please do not change any examples in this PR. All examples should work if our deprecations are correctly put in place.

[Tortar]

I think we can actually change all the examples in this PR for the reason that we already know that all the deprecated code works since all tests still pass, while I have not applied the new way to any examples/tests. I will do commits that do no touch any souce code so that this is preserved (to see only those it is enough to look inside a commit).

[Datseris]

please first run the documentation locally and see that all examples build. before committing any changes to examples.

[Datseris]

In fact, not only that. I argue that this PR in genertal should not alter any examples. The examples should be altered in a new PR after this one is merged in.

[Datseris]

We need to be extra careful with critical changes such as this one.

[Tortar]

ok then let's do it this way

[Tortar]

this is the list of the affected functions signatures by this change:

ABM StandardABM UnremovableABM step! run! offline_run! ensemblerun! ABMObservable abmplot abmplot! abmexploration abmvideo

since the list is very big, controlling the deprecation warnings is a bit hard, so I'm thinking to only warn the users when they use one of ABM, StandardABM or UnremovableABM and in step!, and use a warning which actually says to them that all of these function don't need the passing of the stepping function, maybe in a general way as: "All of the functions definition which expected at least an agent_step! or a model_step! are deprecated, passing these arguments is not necessary anymore. Pass these functions as keywords arguments (agent_step! = your_agent_step!, model_step! = your_model_step!) inside your model definition (ABM(...), StandardABM(...), UnremovableABM(...))."

[Tortar]

Actually I think I can manage to make it work, but anyway I think that saying in each deprecation warning that the change should be applied everywhere the stepping functions are passed seems a good idea to me

[Tortar]

If we want to be extra-sure we should do this verification:

• running old tests with these changes still works

[Datseris]

Is this good to review? I'll take over the Tutorial.md file once this is ready.

[Tortar]

still not good, but soon it should be

[Tortar]

The tests work even in the older version so I think this is ready for review. Running some examples they also work even if I did not update them, so it seems good

[Datseris]

Okay. It will take me a while to review. Let's let this simmer here.

[Datseris]

Okay, I am done here. You should also review the new tutorial and give feedback. I had to update schelling as well, because the schelling example is linked with the tutorial.

[Tortar]

Removing the agents_first from step! broke some tests

[Datseris]

Removing the agents_first from step! broke some tests

Sory about that... Could you please fix the tests?

[Tortar]

Tests are okay now, I looked at the tutorial and it seems fine to me, can we finally merge it? :-)

[Datseris]

We aren't tagging v6 imediatelly after this goes in anyways, so there is no gain in rushing anything. I first need to do a draft for #760 to check if the new model interface makes sense.

[Datseris]

The notice in the changelog of this absolutely massive change is too small. I'll update the changelog and then merge.