Enable mutable state in filter_observations in docs and code

[Wouter1]

Describe the bug

agent_brain.filter_observations requires you to return the new updated state.

Therefore apparently you should not modify the old state.

State itself is not clear about this. I am missing some docu, but it looks like State should be immutable.

An agent must implement filter_observations and return State object.
However there is no good way to create a filtered state.
Because State has no constructor that takes a dict.

You can call State.state_update() but the result of your filter may then be modified through
the decay_val and other things.

Also that call SEEMS to be modifying the state itself, which seems not allowed (see above)

Also notice that internally, state_update uses shallow copies of the provided state.
So if the caller proceeds changing the dict, the state object will be changed in a hidden way.
I think this is all inviting for very hard-to-track issues.

To Reproduce
Create a dict by some filtering of a state, and then try to create a new state object from the dict.

Expected behavior
A simple way to modify the state contents as required

[jwaa]

The state is allowed to be mutated. See the documentation of the filter_observation method.

The filter_observations method in an AgentBrain is meant to allow MATRX users to develop their agent brain in such a way to mimic some cognitive limitations regarding perception (e.g. color blindness). As such the state can be mutated, in fact that is the main reason for that method's existence. Note that "mutation" can occur in two ways: either by altering the state directly, or construct a new state or dictionary that only contains the relevant bits from state. Whatever is easier for a certain agent.

Regarding your point of State making a shallow copy, that is indeed an issue. We will discuss this internally what is best given the default behaviour Python programmers expect (given that shallow copies and point-by-reference are the default in Python).

[Wouter1]

@jwaa thanks for the update.

However I don't see "The state is allowed to be mutated" or anything like that in the documentation. On the contrary it says "returns: A dictionary similar to state but describing the filtered state this agent perceives of the world." suggesting that you must return a modified state in a new dictionary, rather than modify the provided dictionary.

Making a shallow copy and modifying sub-elements amounts to modifying the original. So this is fine only if you explicitly allow modification of the original.

[thaije]

@Wouter1 as described by @jwaa, the idea is that you can modify the provided state variable and cherry pick what you need from it, remove unneeded things, or create a completely new state object. If that was not clear from the documentation, that means we require some updating of said documentation :) I'll add the documentation label and rephrase the issue title a bit to reflect what needs to be changed.

With regard to the shallow copying, that is something which we should fix on the Matrx side of things in my opinion, such that the user doesn't have to worry about it and can just edit their state without issues.

[thaije]

I tested the issue and there indeed seems to be a problem with shallow copies. I tested it by making a change within an agent on the state passed via its filter_observations function like so:
self.state['human2_79']['class_inheritance'].append("test")
Then checking `gridworld.registered_agents['human2_79]['class_inheritance'] confirms that 'test' has also been added over there.

So a fix could be to make a deepcopy of the state before passing it to the agent, so I tried some things to get an indication of te performance hit. I added some code in the _get_action() from the agent_brain in the vis_test.py default MATRX case:

a = time.perf_counter()
    state2 = >make deepcopy of self.state<
    b = time.perf_counter() - a
    print(b)

I basically tried the list from here.

I first tried copy.deepcopy() but that is incredibly slow.
1 agent for 1 tick for a state with 81 obejcts took 10ms / 0.01s on average. So if you have 10 agents that is 0.1s per tick spent only on copying..

Better was ujson.loads(ujson.dumps(a)) which took around 5ms / 0.005s on average per agent / tick / state with 81 objects.
But that would still amount to a maximum of 20 ticks per second if you have 10 agents.

Most efficient might be to have the user create their own state and cherrypick what they need, however that would just put the optimization problem with the user. If the user makes almost no changes in the filter_observations, the entire state will still have to be copied. @jwaa do you have any ideas on how to solve this?

[thaije]

@jwaa in MATRX v3, how did you solve this problem?

[jwaa]

Not sure, have to check. But there is a good chance v3 is not at this point. I'm just implementing the agents' State from the world/god State using Sensors. However, I think a copy of the values would do the trick. This prevents the nees for a deepcopy later on as the State's values are copied (e.g. their references cut) from the actual agent properties.

[thaije]

However, I think a copy of the values would do the trick. This prevents the nees for a deepcopy later on as the State's values are copied (e.g. their references cut) from the actual agent properties.

@jwaa this would be a solution for MATRX v3 you mean right? And not possible in v2?

[jwaa]

Yes, v3 only. We could also try and apply it to v2 though that version does not have an easy access point for this, making it potentially slow :(

Your jsonify solution is easier and probably faster, or just as fast.