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

Fix for underlying MDP not preserving state-dependent action space #430

Closed AlexBork closed 1 year ago

AlexBork commented 1 year ago

Fixes JuliaPOMDP/POMDPs.jl#429 as proposed in the issue.

zsunberg commented 1 year ago

Hi @AlexBork Thanks for your contribution! Can you also add a test to test/model_tools/test_underlying_mdp.jl that exercises this line of code? It is always good to at least exercise every code branch in the tests so that we can spot errors and make changes with confidence.

I suggest something like the following

struct Issue429POMDP <: POMDP{Int, Int, Int} end
POMDPs.actions(m::Issue429POMDP, s) = [1,2]
@test actions(UnderlyingMDP(Issue429POMDP(), 1)) == [1,2]
AlexBork commented 1 year ago

Hey @zsunberg, I added the test as requested. Let me know if there are any other requests.

zsunberg commented 1 year ago

Thanks @AlexBork Looks good to me!

For your benefit, note that you could have used:

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

but it is not clear whether that is actually better than what you have.