JuliaPOMDP / GenerativeModels.jl

Extension to POMDPs.jl for problems with a generative model (eliminates the need for distributions)
Other
7 stars 2 forks source link

Errors make it seem like you need transition() #9

Closed zsunberg closed 7 years ago

zsunberg commented 7 years ago

Because of the default implementations, if one of these functions is missing, the error we will end up getting is "no method matching transition(...)" making it seem like the problem writer needs to implement transition(). This is misleading. How can it be fixed?

zsunberg commented 7 years ago

Would it be appropriate to do

if method_exists(transition, Tuple{typeof(mdp), S, A})
    # use default implementation of transition
else
    # throw method error
end

in some of the methods, or would that cause a performance hit?

mykelk commented 7 years ago

Not sure, but it kind of doesn't make me feel very good. Does it make sense to provide a default implementation that errors out when it is called?

zsunberg commented 7 years ago

Yes, it does not feel very good. No it does not make sense to provide a default implementation that errors out [1]. We must provide a default implementation that uses transition() if we want models defined with explicit probability distributions to work with MCTS and other gen model solvers without requiring the additional definition of the functions in GenerativeModels.

[1] By the way, I don't think we should ever provide default implementations that error out anymore in any packages. Empty generic functions are better now for several reasons that we can discuss elsewhere.

zsunberg commented 7 years ago

Maybe the best way to do this is to provide no silent defaults, but to provide a function that does it, for example GenerativeModels.implement_for(::Function, ::POMDP) that provides default implementations if necessary/possible.

zsunberg commented 7 years ago

JuliaPOMDP/POMDPs.jl#121 allows this to be fixed.