Agents.jl icon indicating copy to clipboard operation
Agents.jl copied to clipboard

Should we just go ahead and use `SVector` in position and velocity fields of ContinuousSpace...?

Open Datseris opened this issue 2 years ago • 4 comments

Alright, so, plenty of times in many use cases of ContinuousSpace, it becomes an obvious pain to use NTuple instead of SVector. So, should we just go ahead and change the types to static vectors? Problem is, I am not sure how breaking this is. Unless a user for whatever reason annotatted the position type, everything the user did before will work now as well. This includes changing velocity via a tuple, agent.vel = (1, 2) would still work even if vel isa SVector.

For example, this problem came when making the Flocking example, in the discussion in #671 , in the integration example with CellListMap.jl, the bacteria example, I mean, practically everywhere with ContinuousSpace.

Datseris avatar Aug 05 '22 16:08 Datseris

@mastrof perhaps you want to start a PR that does this change, so we can get an estimate of what things may be breaking?

Datseris avatar Aug 05 '22 16:08 Datseris

Unfortunately, I think this is breaking, simply because before the @agent change our API allows writing structs by hand, which means people probably did annotate the type. I know I've done it, simply because I don't like the squiggly underline VSCode puts whenever I use @agent. Bar that, I think most if not all code should just work with SVector{2} as well, since .+ and .* do the same thing in both cases. The only exception off the top of my head is using add_agent! and passing NTuple for the agent position.

We could just use a union type internally and typecast it to SVector every time we modify the struct

AayushSabharwal avatar Aug 06 '22 05:08 AayushSabharwal

Sure, happy to start a PR, should be able to do it tomorrow (actually today, since midnight just clocked here). So, if I get what you are after, I'll change the ContinuousAgent field types from NTuple to SVector, and consequently all the function signatures that expect a NTuple from a ContinuousAgent will ask for a SVector instead.

For the limited knowledge I have of the library, spaces/continuous.jl and spaces/utilities.jl seem to be the only "obvious" affected files (+ Agents.jl just to import StaticArrays).

mastrof avatar Aug 06 '22 22:08 mastrof

and consequently all the function signatures that expect a NTuple from a ContinuousAgent will ask for a SVector instead.

Furthermore you need to add SVector to ValidPos. I don't remember where the definition of ValidPos is, I think in model.jl.

Datseris avatar Aug 07 '22 08:08 Datseris