airbus / scikit-decide

AI framework for Reinforcement Learning, Automated Planning and Scheduling
https://airbus.github.io/scikit-decide
MIT License
142 stars 28 forks source link

Wrong python state type in observability.py #200

Closed fteicht closed 11 months ago

fteicht commented 2 years ago

In observability.py the type of states is mentioned as D.T_state everywhere in the file whereas the of observations is always D.T_agent[D.T_observation].

See for instance the following member function: https://github.com/airbus/scikit-decide/blob/4a8542dde3f6e43cd67d463e50513ae5b54cc0b7/skdecide/builders/domain/observability.py#L102

I think the correct type should be D.T_agent[D.T_state] everywhere for consistency reasons. As an additional argument, if we don't align the type of states with the type of observations (i.e. based on D.T_agent[...], it would mean that we can infer the probability of the observations of all the agents based on the observation of a single agent. Which does not seem to be reasonable.

neo-alex commented 2 years ago

I would rather keep D.T_state as-is since I believe many domains could have a single "ground truth" state from which agent-specific observations are derived in the general case. And for domains where it would more natural/appropriate to store an agent-specific state, nothing prevents the domain creator to set T_state = Dict[...] where each key would correspond to an agent ID (maybe we should create a test scenario to make sure this is indeed feasible without any issue).

nhuet commented 11 months ago

If @fteicht agrees with @neo-alex argument, i think we could close this ?