Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FullyObservablePOMDP broken obsindex #480

Closed
WhiffleFish opened this issue Apr 26, 2023 · 5 comments
Closed

FullyObservablePOMDP broken obsindex #480

WhiffleFish opened this issue Apr 26, 2023 · 5 comments

Comments

@WhiffleFish
Copy link
Member

obindex definition wrongly assumes that parametric type order for FullyObservablePOMDP is {S,A,...} when it really is {M,S,A}. Consequently, a method error is always raised.

struct FullyObservablePOMDP{M,S,A} <: POMDP{S,A,S}

POMDPs.obsindex(pomdp::FullyObservablePOMDP{S, A}, o::S) where {S, A} = stateindex(pomdp.mdp, o)

@lassepe
Copy link
Member

lassepe commented Apr 26, 2023

Isn't the fix just to define

POMDPs.obsindex(pomdp::FullyObservablePOMDP{<:Any, S, A}, o::S) where {S, A} = stateindex(pomdp.mdp, o) 

@lassepe
Copy link
Member

lassepe commented Apr 26, 2023

In fact, why are there any type parameters at all? Seems like we can just drop them in obsindex

@WhiffleFish
Copy link
Member Author

Yeeeep it does seem unnecessary. The only reason I see to keep the state type parameter is to assert that the observation really is a valid state, preventing it from either failing silently or relying on stateindex to catch the type error.

@zsunberg
Copy link
Member

Yeah, we should just get rid of S and A. We might want to accept observations that are not of type S, for example when S is a StaticVector but the user inputs a Vector.

This is an ancient leftover from the early days of Julia where you either needed those parameters or the coders (i.e. me) did not realize you didn't need them.

@WhiffleFish
Copy link
Member Author

fixed by #485

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants