JuliaDynamics / Agents.jl

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

Implement multiagent macro #957

Closed Tortar closed 8 months ago

Tortar commented 8 months ago

Fixes #947

Because I'm a bit crazy I've already implemented it :D

(Notice that the more complicated example in #947 works and also the rabbit_fox_hawk example I modified)

Tortar commented 8 months ago

Now even this more complicated compactification works

@compact struct Animal{T,N,J}(GridAgent{2})
    @agent struct Wolf{T,N}
        energy::T = 0.5
        ground_speed::N
        const fur_color::Symbol
    end
    @agent struct Hawk{T,N,J}
        energy::T = 0.1
        ground_speed::N
        flight_speed::J
    end
end
Tortar commented 8 months ago

Seems kind of fine to me!!

Tortar commented 8 months ago

Let's rename it @multiagent then

(yes, all tests pass locally, and strangely also when we merge, I think this problem will disappear in future PRs maybe)

Tortar commented 8 months ago

Renamed and increased tests, let me know if you have some more suggestions

Tortar commented 8 months ago

I have actually a bigger plan for this macro and the different macro I'm creating in https://github.com/Tortar/StructSumTypes.jl.

I will make a new package out of them, I think it will be pretty cool

but for now let merge this in as it is in Agents.jl when it is ready

(I rapidly tested the macro in https://github.com/Tortar/StructSumTypes.jl and in some cases it as memory efficient as a collection of different types with a little runtime penalty (40% slower) than the macro here)

Tortar commented 8 months ago

Nonetheless I need to change the access to a.type in kindof(a) to be compatible with future development, but this shouldn't be a problem

Datseris commented 8 months ago

Perhaps you can consider starting these packages in the JuliaDynamics organization. The way things are right now, we are adding some "private" dependencies here, but if in the future you work elsewhere and stop maintaining these, these packages will be unreachable. You have access in the org to start them here, and in the package repo you will still be shown as the one with all the commits, so this gives you the credit while it also future-proofs Agents.jl from depending on dependencies that may become unreachable.

Tortar commented 8 months ago

Good idea, transferred both :+1:

https://github.com/JuliaDynamics/IteratorSampling.jl https://github.com/JuliaDynamics/MixedStructTypes.jl

codecov-commenter commented 8 months ago

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (6bb5346) 85.23% compared to head (c704a32) 67.54%. Report is 2 commits behind head on main.

:exclamation: Current head c704a32 differs from pull request most recent head 6b8ea51. Consider uploading reports for the commit 6b8ea51 to get more accurate results

Files Patch % Lines
src/core/agents.jl 93.10% 4 Missing :warning:
ext/AgentsVisualizations/src/inspection.jl 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #957 +/- ## =========================================== - Coverage 85.23% 67.54% -17.70% =========================================== Files 36 48 +12 Lines 2533 3269 +736 =========================================== + Hits 2159 2208 +49 - Misses 374 1061 +687 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Tortar commented 8 months ago

Hi George, should be okay now if you want to take a look

Tortar commented 8 months ago

I updated the performance tips, I don't anymore mention the problem with working many types, I think our macro is good enough for many use cases already.

However, I don't consider the problem totally solved, and I consider https://github.com/JuliaDynamics/Agents.jl/issues/841 and its variations worth exploring.

E.g. our macro falls short when the types have a memory footprint very different from one another, because when we use the memory efficient version we are actually occupying anyway the memory of the agent with highest memory footprint (instead of the sum of their footprints which happens with :opt_speed) which can be sometimes too much anyway

Datseris commented 8 months ago

great, I will review this on the weekend!

Datseris commented 8 months ago

Okay I will review this now and tomorrow. If you are around, any chance I can get a summary of this PR in a comment here?

Datseris commented 8 months ago

all tests fail, I guess this is the "known" problem and everything passes locally?

Tortar commented 8 months ago

In practice I implemented the multiagent macro where I also give to the user the possibility to choose between a more optimized version for speed or for memory by passing :opt_speed or :opt_memory as the first argument of the macro, by default it optimizes speed. Then I extended internals when needed, and I edited the branching_faster_than_dispatch.jl file so that to see how the two versions perform, you can find the results at the end of the file.

all tests fail, I guess this is the "known" problem and everything passes locally?

exactly, also strangely when we merge tests pass instead

Datseris commented 8 months ago

@Tortar can you remind me what is the performance comparison of this method and the standard Union method that we used before? How much faster this is here? There was some benchmarks somewhere , right?

Tortar commented 8 months ago

Yes, I updated two benchmarks in this PR, I described the results for variable_agent_types_simple_dynamics.jl in the other review comment, for the results of the other, see the bottom of branching_faster_than_dispatch.jl

Datseris commented 8 months ago

right, the first file does not have any comments with the results, but from the second one https://github.com/JuliaDynamics/Agents.jl/blob/f750aeffbcea1d284df72b86973a10f6834b64f4/test/performance/branching_faster_than_dispatch.jl#L190-L193 it appears that performance is up to 7x which is fantastic.

Datseris commented 8 months ago

i'll merge this in when you adress the comments for the performance tips documentation. then i'll work on a re-worked tutorial and docs.

Tortar commented 8 months ago

Docs updated!

Datseris commented 8 months ago

oh, btw, what impliciations does this have for the Event queue abm? does it make it much faster as well? I Guess we should have a specialization there if there are no different agent types?