Agents.jl: Regarding #176 : warn, not error

[Datseris]

Regarding the PR #176 , we should warn, and not error, if any of these checks fail, e.g. if agent type is not concrete, etc.

(besides the id field being first and an integer, and the pos field being appropriate. There we should always error)

There is a simple reason: some users may want to provide Union{Tree, Woodcutter} as their agent type, i.e. create a mixed agent-based-model. I guess @Libbum you would be one of those people, given the figure in #141 already uses 2 types of stuff.

Of course, you could do this by creating two agent-based models, but maybe you want to use the API more directly, including data collection, etc.

---

@Libbum once again I am sorry for merging stuff before thorough thinking about them.

[Libbum]

(No worries, gives me time to rebase between PRs! Things are moving fast here)

Sure, that's a great idea. I'll fix that up.

At some stage I should write a mixed model example too.

[Libbum]

I've slept on this and would propose something a little different.
This current suggestion would provide warnings all the time if users do make mixed models. To fix this, we could do one of the following:

1. Add a warn=true keyword to ABM that removes warnings when set to false such that users who are aware of the issues with their agent can suppress the messages.
2. Continue to fail on all checks. Add an additional check such that all types in the Union are subtypes of AbstractAgent and adhere to our current requirements.
3. Swap errors to warnings. Add the same additional check as (2). Suppress the isconcretetype warning if we identify a mixed agent Union.

Which of these solutions we implement would depend on use cases other than mixed models. I can't think of one at the moment, so my bet is on (2). Keep it tight, make sure everything is as optimal as possible, revisit if other use cases come up. I don't think (1) is the best way, since there's the possibility of new users copying code from more experienced users and not following the guidance of the warnings. (3) is the middle road, I'd be fine with that too.

Thoughts?

(On a personal note, I'd want to use mixed ABMs in a way that each community of agents interact themselves in one way, but with other communities differently. But they do interact. So creating two separate models wouldn't obtain the same dynamics)

[Datseris]

Number 1 is the way to go.

[Libbum]

Implementing this now, but it may not be as clear cut as swapping all to warnings

Checks:

• concrete: change to warning, no issues
• immutable: change to warning, will error if anything happens in update_agent! later
• first field id: keep as error
• second field pos: keep as error
• id, pos, vel type checks: these may as well error too, since they'll fail later on in the run if not at this point

So, other than the concrete check, should we be actually swapping any of these?

[Datseris]

Very nice way to present the information. I give okay to everything except the velocity check. It should not error. If vel exists but is not of type NTuple, then this error should be put into move_agent!, not the ABM() instantiation function.

(so that users may use vel field on their own for specific applications)

[Libbum]

Great. Almost done with this then.

Just to confirm that last point, should we be dropping the velocity type check completely here? Or do you mean keep it but demote it to a warning?

[Datseris]

keep it as a warning, since we will have a verbose keyhword anyway that silences all warnings if true