gen to replace generate_x

[zsunberg]

I am trying to write an extension for constrained MDPs and realizing the limits of the generate_x paradigm that we have been using so far.

In addition to the next state, observation, and reward, I need to return a cost c. So solvers would use something like generate_sorci. This seems to be moving beyond the happy limits of our habit of naming generate functions like this.

I think we should replace this with a single function gen.

Simple Description

Problem-writer's perspective

Problem writers would implement

gen(m, s, a, rng)

which would return a NamedTuple with fields sp and r, along with o for POMDPs, and possibly others like i for info or c for the constraint cost.

Solver-writer's perspective

Solver writers would use, for example

sp, o, r = gen(Val((:sp,:o,:r)), m, s, a, rng)

The first argument specifies what is needed and the order.

Advanced Usage and Implementation Details

Implementation

The function

gen(::Val{T}, m, s, a, rng) where T<:NTuple{N<:Int, Symbol}

will be implemented as a generated function in POMDPs.jl to switch around the arguments and throw helpful errors if things are missing.

Optimization by solver-writers

If advanced problem writers want to optimize, they can implement a gen(::Val{T}, ...) directly. For example, if they want to use a particle filter and don't want to generate extra observations, they would implement gen(::Val{:sp}, m, s, a, rng).

Extension

There will be a mechanism to hook into gen(::Val{T}, ...) for extensions to provide helpful error messages, etc. for extensions like constrained MDPs.

Advantages

• Simplicity for problem writers
  • They only need to implement one function.
  • They only need to understand NamedTuples, and the error message when they don't would be straightforward.
  • They don't need to understand value types.
• The transition between generate_x and gen with deprecations, etc shouldn't be too painful.
  • From the solver-writer side, @deprecate should work
  • From the problem-writer side, it should be fairly easy to have a default implementation of gen that warns once about the deprecation, and then uses generate_sor
• I think it would be fairly easy to generate efficient code that throws helpful error messages if things are missing from the named tuple.

Disadvantages

• In things like particle filters tons of extra observations would be generated and thrown away by default (of course this can be avoided by advanced users).
• Solver writers need to understand value types

Open questions

• Should there be gen(::Val{:o}, m::POMDP, s, a, sp, rng)?
• Should reward be used to generate r if it is not already in the NamedTuple? (I think yes)
• Should we actually use value types, or just tuples? (I think use value types)

[zsunberg]

The more I think about this, the more I think it is a good idea. Does anyone else have opinions? I am going to start on a test implementation to see how smooth the transition will be.

[mykelk]

Sounds good.

[zsunberg]

I have implemented this in the genvarmin branch, and it seems to work pretty well. Notes:

• With this change, (almost) everything will still work out of the box with deprecation errors
• After this change, to get rid of dep. warnings (and the associated performance penalties), other packages need the following:
  • POMDPSimulators will need a major overhaul
  • All solvers that use the generative interface will need minor (find-and-replace) updates
  • All problems that use the generative interface will need minor (find-and-replace) updates
  • The tutorials will need minor (find-and-replace) updates
• Documentation is not yet complete

[lassepe]

Regarding the disadvantages that you mentioned above:

1. We can still implement the default belief updaters to use a version that does not generated unneeded observations, right?
2. I think value types are fine and too hard of a concept to understand.

[MaximeBouton]

This sounds like a good idea! We have to make sure that the fact that it should output a named tuple is well documented .

[zsunberg]

We can still implement the default belief updaters to use a version that does not generated unneeded observations, right?

Yes, particle filters will use gen(Return(:sp), m, s, a, rng). Sometimes the compiler can automatically optimize out unnecessary computations; sometimes it will be slightly more efficient to implement gen(::Return{:s}, ...) directly: https://github.com/JuliaPOMDP/POMDPs.jl/blob/genvarmin/docs/src/generative.md#performance-considerations

[zsunberg]

This design directly violates the principle that generated functions should not observe global state (https://docs.julialang.org/en/v1/manual/metaprogramming/index.html#Generated-functions-1). I guess I need to rethink this.

[zsunberg]

If anyone has any ideas how to implement this without @generated functions, let me know, because I do not.

[lassepe]

Just to be clear on this: the observed global state is the rng, or which part of this is the problem?

[zsunberg]

No, it's the genvar_registry, which is accessed with genvar_data() here https://github.com/JuliaPOMDP/POMDPs.jl/blob/genvarmin/src/gen_impl.jl#L41

I am also definitely open to completely new designs (but we probably wouldn't be able to release by AA228)

[zsunberg]

One of the things that makes this more difficult is throwing helpful errors. As of 0.7, when a generate function is missing the error says something like "no method for generate_s(...). you should also consider implementing transition(...)". An easier-to-implement version would throw an error like "no method for transition(...). Did you forget to implement gen?". I'm afraid the latter will lead people down the wrong road, but maybe it's not as important now and even less-so in the future with more QuickPOMDPs models

[lassepe]

No, it's the genvar_registry, which is accessed with genvar_data() here https://github.com/JuliaPOMDP/POMDPs.jl/blob/genvarmin/src/gen_impl.jl#L41

I feel like there must be a way to do this. Maybe this is a good question for discourse.

Does the genvar_registry ever need to change dynamically? The only thing that I can think of (disclaimer: non-converged thought) is that we might be able to make the registry a const NamedTuple and if people want to add something they have to declare another const NamedTuple (containing the additional registry entries and declared before the definition of the generated function(?)) for which the name is known. We can check for existence of this variable and use the joint of both const NamedTuples as a registry. But all of this seems super ugly, a weird workflow and hard to understand (if we can make it work at all).

[zsunberg]

It can change when another package adds a genvar. e.g. ConstrainedPOMDPs.jl might add :c

[zsunberg]

https://discourse.julialang.org/t/alternative-to-redefining-a-generated-function-to-observe-subsequently-static-state/28226

[Shushman]

I don't see a PR for the genvarmin branch. Were you planning on doing that @zsunberg ?

[zsunberg]

Yeah, this afternoon I will submit a PR with this and the new DBN structure stuff

…

On Sun, Sep 1, 2019 at 10:54 AM Shushman Choudhury ***@***.***> wrote: I don't see a PR for the genvarmin branch. Were you planning on doing that @zsunberg <https://github.com/zsunberg> ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#240?email_source=notifications&email_token=ABALI22SV5VUCDIOC3BMYT3QHP6VJA5CNFSM4HPRHYB2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5UHO4A#issuecomment-526940016>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABALI24NS7TK6ME4LN2N33TQHP6VJANCNFSM4HPRHYBQ> .