search

JuliaPOMDP / POMDPs.jl

MDPs and POMDPs in Julia - An interface for defining, solving, and simulating fully and partially observable Markov decision processes on discrete and continuous spaces.
http://juliapomdp.github.io/POMDPs.jl/latest/
Other
657 stars 100 forks source link

State-dependent action spaces in POMDP are not transfered to underlying MDP #429

Closed AlexBork closed 1 year ago

AlexBork commented 1 year ago

The call of an underlying MDP for a POMDP does not preserve state-dependent action spaces, i.e. calling actions(UnderlyingMDP(pomdp), s) for a POMDP with state-dependent action space yields a wrong result.

Adding 

POMDPs.actions(mdp::UnderlyingMDP{P, S, A}, s::S) where {P,S,A} = actions(mdp.pomdp, s)

to ModelTools/underlying_mdp.jl fixes the problem. I propose adding the fix to the main repository.

zsunberg commented 1 year ago

Hi @AlexBork , thanks for reporting. One issue is that actions(m::POMDP, s) is ill-defined because the agent can't typically tell which state it is in; Thus, actions(m::POMDP, s) instead we use actions(m::POMDP, b). So your proposal might lead to errors if the person who defined the POMDP expects a belief instead of a state.

Another option would be for you to define

POMDPs.actions(::UnderlyingMDP{<:YourPOMDP}, s)

What do you think about that?

If you still think that adding the method you proposed to POMDPTools is a better solution, I'd be happy to accept a PR!

zsunberg commented 1 year ago

Actually I guess that actions(m::POMDP, s) is advertised as part of the interface (https://github.com/JuliaPOMDP/POMDPs.jl/blob/52ee24c4e98da73f8c67f953e373d25fdd77e77e/src/space.jl#L19), so we should make your proposed change.

Can you submit a PR (this would be great practice if you haven't done it before!), or should I make the change?

AlexBork commented 1 year ago

Hey @zsunberg, thank you very much for the swift answer. I will submit a PR containing the change.