JuliaPOMDP / MCVI.jl

Other
5 stars 3 forks source link

nonstandard use of pdf() #8

Closed zsunberg closed 7 years ago

zsunberg commented 7 years ago

In subspace.jl, line 35 pdf is used with state and observation arguments.

This makes sense, but it is nonstandard since the first argument to pdf should be a distribution. I think we should change this.

The first question to ask is could we possibly replace it with

pdf(observation(pomdp, a, particle(ss,i)), obs)

?

i.e. does an MCVISubspace correspond to a specific action, or not? @rejuvyesh do you know the answer to this?

If the answer is no, we should consider fixing JuliaPOMDP/POMDPs.jl#143 .

Another consideration is that, in some cases, it is a pain to define distributions when you only need the weight. I addressed this in ParticleFilters.jl with the obs_weight function that defaults to calling pdf(observation(pomdp, a, sp), o). I think we should do something similar here (if it was me, I'd just import obs_weight from ParticleFilters), but it should not be called pdf because it is not operating on a distribution.

rejuvyesh commented 7 years ago

Each MCVISubspace does correspond to an action in the sense that you move to that subspace because of some selected action (and reuse the MCVISubspace if the action key exists).

Definitely. It's been a while. This definitely should be obs_weight or something like that. I was probably trying to reuse POMDPs terminology at the time.

Honestly, sometimes I think I should rewrite this whole thing again. Fairly embarrassed of my past self.

zsunberg commented 7 years ago

Could an MCVISubspace be reached from multiple actions, or only 1? If it's only 1, we can just add that action as a field in the MCVISubspace and then use it on line 35.

Haha, @rejuvyesh don't be embarrassed! When we started POMDPs, we were trying to do something pretty new, in a new language with a lot of unknowns - there will always be some imperfections in that situation. I feel the same way about MCTS - it's extremely slow compared to what it could be. But I think this is a good first cut. Hopefully someone will start using it and want to rewrite/significantly revise it in the future.