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

Replacing GenerativeModels.jl by a single specification in POMDPs.jl #10

Closed juliohm closed 7 years ago

juliohm commented 7 years ago

From my experience using the framework for the first time, I had the impression that the current separation of POMDPs.jl and GenerativeModels.jl could be rethink. I will give you an example of the issues I had with the current approach and why I think we could simplify the interface.

My generative model is super expensive and I cannot afford running it multiple times. Because there are many variants for generate_s, generate_sor, ... it is hard to enforce a rule on what should be implemented at the call site in the solvers and/or belief updaters. In my case, I had to implement both generate_s and generate_sorto get POMCP and the SIR particle filter working.

It would be easier to just have one version like in the textbook G(s,a) possibly passing an optional argument G(s,a,opt=nothing) that stores external parameters required by the underlying complicated solver. Currently, I am appending these external parameters to my state, but it is tricky to get things right, maybe I am doing something completely wrong?

My proposal consists of just having a single specification for a generative model in POMDPs.jl. This would reduce the complexity of maintaining an additional package just for this concept and would keep things clearer for solver writers and users.

Can you please share your thoughts and rationale for the existence of this package?

mykelk commented 7 years ago

Thanks for taking the time to provide feedback on improving the package. @zsunberg is looking at ways to remove the need for generate_ functions and greatly simplify the interface. See https://github.com/JuliaPOMDP/POMDPs.jl/issues/111

zsunberg commented 7 years ago

Hi Julio, thanks so much for your feedback! It is helping us to make changes and think through our design decisions.

First of all, @mykelk meant that we are getting rid of the create funcitons, not the generate functions. Some form of generative model interface is definitely staying.

Having both generate_s and generate_sor implemented does not mean that the generative model is called twice as often - the two forms are just used in different places. In the SIR particle filter, only state simulations are needed, so, in my opinion it would be weird to ask the problem writer to return a reward and observation if they don't need to. generate_s can also be used inside of generate_sor so no one should ever need to have the code for generating the next state in two places. In fact, I would normally just encourage a problem writer to write generate_s, generate_o, and reward separately, and GenerativeModels has a default implementation of generate_sor that combines them. But, since your python code generates s, o, and r simultaneously, it makes sense to have generate_sor and generate_s separately.

There are several reasons that we didn't go with a G(s,a,opt=nothing) approach like the one you mentioned. Some reasons are given below.

1) We think it is very important to have separate objects that represent both the problem and the state.

  1. The problem object is important (as opposed to just having the dynamics defined by a function G) because it is much easier to represent different variations on the same problem (i.e. a car that weighs 1 ton vs a car that weighs two tons) with different objects rather than different functions. Moreover, functions are difficult to serialize for storage/moving over a network, while objects are easy.
  2. We have found that, while it sometimes takes some thinking to figure out the best way to organize everything, separating the state from the problem (as opposed to having just an environment like in openai gym) is beneficial. It often results in more clear reasoning and communication about the problem and organizes it in a way that is easy for the planners we use. We have used the interface to represent some very complicated problems, and we think that the problem-state separation is an advantage in the long run. 2) The sor is important. Julia can do aggressive optimization if it knows the return types of functions, and appending these letters to the end of the function name is the easiest way to make sure the problem writer, simulator writer, solver writer, and updater writer are all on the same page. As explained above the problem writer should never have to have the same code in multiple places, and I can't think of another way to organize it where the solver writer doesn't have to demand more or less than he needs and the problem writer doesn't have to implement more or less than she can. 3) As we have seen with the create_ functions, having optional arguments in a function whos methods will be written by someone else is really confusing and usually causes more pain than it's worth.

The original reason for the separation of this package from POMDPs was so that a few of us could implement it and use it without having to debate every implementation detail with the rest of the POMDPs.jl developers :) and to keep the POMDPs specification smaller. In fact, POMDPs.jl can stand on it's own completely without GenerativeModels.jl. A clever coder could implement a generative model with the transition and rand functions. It is really just a very useful convenience package. It is also sometimes useful to have it separate conceptually because it is an alternative to implementing transition, observation, rand, etc. It is easy to refer to it as a separate package when communicating about it. I don't think there is much extra overhead to maintaining it separately (in fact it might be easier because everything is less monolithic that way, and I think much of our documentation problems stem from just how big and flexible this thing is). All that being said, I wouldn't be too opposed to merging it in to the main package. More people probably use generate_ than transition.

zsunberg commented 7 years ago

If you really like doing things with functions and think others would as well, you could make something like

type FunctionPOMDP <: POMDP
    G::Function
    # other functions
end

generate_sor(p::FunctionPOMDP, s, a, rng) = p.G(s,a)

# etc.
zsunberg commented 7 years ago

Also, I think much of the pain that is experienced by new users of POMDPs is because I made debugging difficult by trying to do fancy things with errors and default implementations (e.g. @pomdp_func and #9). A lot of this will be fixed soon, and GenerativeModels.jl will seem much simpler and clearer when it's easier to debug.

juliohm commented 7 years ago

Hi Zach, thank you for the clarification, based on your comments, maybe we could achieve a simpler solution with Functors: http://docs.julialang.org/en/release-0.5/manual/methods/#function-like-objects. These are objects that hold state and can be called as functions.

If not functors, I still think that a single function generate_sor is better than multiple flavors. Better avoid bugs that may manifest themselves from all the combinations generate_:(s,o,r). Combinatorics is a tricky topic in any context. :blush:

My personal opinion is that a separate package for holding the concept of a function (that happens to be a simulator) is not very advantageous over having this function specified in POMDPs.jl where it is supposed to be used anyways. Perhaps a submodule in POMDPs.jl if you need encapsulation?

Maybe the question we can ask ourselves is the following: Can the concepts in GenerativeModels.jl be used without POMDPs.jl? If the the answer is yes, then you have a reason to keep it in a separate package.

mykelk commented 7 years ago

I think the idea with POMDPs.jl is to make it as simple as possible like MathProgBase.jl. JuMP.jl and Convex.jl can sit on top of it.

juliohm commented 7 years ago

After discussing today with Zach, I changed my mind about the solution with a single function generate_sor. He clarified some details and it is indeed fruitful to have all of them generate_s, generate_sor,... available. I still think the user experience could be improved by making the generative model specification a part of POMDPs.jl. Can the GenerativeModel.jl be used as a standalone package?

mykelk commented 7 years ago

I think GenerativeModel.jl can be used just by running using GenerativeModel.jl, but running it requires POMDPs.jl, but I don't think the user needs to know that.

juliohm commented 7 years ago

I am just trying to understand what do we gain by doing:

using POMDPs
using GenerativeModels

instead of

using POMDPs
mykelk commented 7 years ago

I think we ought to be able to just do using GenerativeModels. Just like how we can do using JuMP without the user having to worry about MathProgBase.

juliohm commented 7 years ago

But when is using GenerativeModels by itself a sensitive thing to do? I am not very familiar with JuMP.jl, but it seems like we can use it as a standalone package with the accompanying solvers?

Or maybe the intention is to only do using GenerativeModels without mention to POMDPs.jl?

mykelk commented 7 years ago

According to https://github.com/JuliaOpt/JuMP.jl/blob/master/src/JuMP.jl, it imports MathProgBase. I'm inclined to follow the JuMP kind of model for this since those developers have thought quite a bit about its design and are exceptionally familiar with Julia---but of course, we're open to other ideas as well.

juliohm commented 7 years ago

I just felt a bit confused by the time I was using the package for the first time, didn't understood the rationale for having the specification of generative models standing alone :blush: I will close the issue for now, but feel free to reopen or brainstorm it more.