Compatibility with the POMDPs v0.8 release #56

Shushman · 2019-09-26T21:35:12Z

No description provided.

coveralls · 2019-09-26T21:50:32Z

Coverage decreased (-0.3%) to 65.806% when pulling 4d5dc46 on pomdps-v0.8-compat into e6fb186 on master.

MaximeBouton

Shouldn't we remove the legacy grid world now?

zsunberg · 2019-09-27T01:00:33Z

I don't think we should remove legacy grid world - there are a bunch of tests that rely on the exact value function for it. We should, however remove the old GridWorld alias for the LegacyGridWorld that has been deprecated. I also think we can remove the visualization of LegacyGridWorld which can remove the TikzPicture dep

zsunberg · 2019-09-27T01:00:49Z

thanks for working on this!

src/CryingBabies.jl

zsunberg · 2019-09-27T01:46:20Z

Project.toml

@@ -18,8 +18,9 @@ StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
 StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91"

 [compat]
-POMDPSimulators = "< 0.3.0"
-julia = "1"
+POMDPSimulators = "0.3.0"


This line should be POMDPSimulators = "0.3". Does the difference make sense?

Ah, so it subsumes any 0.3.x versions?

Project.toml

zsunberg · 2019-09-27T02:29:14Z

src/TMazes.jl

@@ -219,7 +216,7 @@ function stateindex(maze::TMaze, s::TMazeState)
    end
 end

-function generate_o(maze::TMaze, s::TMazeState, rng::AbstractRNG)
+function gen(::DDNOut{:o}, maze::TMaze, s::TMazeState, rng::AbstractRNG)


It seems like the observation only depends on sp, so we should just implement observation(::TMaze, sp) and no other methods of observation or gen

Would this observation return a BoolDistribution?

src/TigerPOMDPs.jl

zsunberg · 2019-09-27T02:33:08Z

test/crying.jl

-    # test generate_o
-    o = generate_o(problem, true, MersenneTwister(1))
+    # test gen(::o,...)
+    o = gen(DDNOut(:o), problem, true, MersenneTwister(1))


Is this because it is an initial observation? I thought solvers etc. should call DDNOut

src/TigerPOMDPs.jl

…MDPModels.jl into pomdps-v0.8-compat

Shushman · 2019-09-27T17:37:27Z

I'm going to be unavailable for most of the rest of today. The outstanding items seem to be
(i) Removing the deprecated GridWorld ref and the legacy visualization
(ii) Changing the gen in src/TMazes.jl to the corresponding observation

If this is time-sensitive, could one of @zsunberg or @MaximeBouton see this through? Otherwise, I can get to it tomorrow.

MaximeBouton · 2019-10-09T21:27:17Z

Is there anything left to fix here?

zsunberg · 2019-10-09T21:33:41Z

I'm planning to check through it this afternoon and see.

Do we want to add initialobs and wait for POMDPs 0.8.1 (maybe a couple days)?

MaximeBouton · 2019-10-09T21:38:06Z

So far it is using observation(m, s) for TMaze, I thought initialobs would call this by default so no need to implement it?
Or are you saying we should implement initialobs for some of the problems?

zsunberg · 2019-10-09T21:42:33Z

Oh, ok, yeah as long as we don't have gen(DDNNode{:o}, m, s, rng) (our original initial observation idea for 0.8), then I think it is fine to go ahead. Let me take a couple minutes to check through it (Does it look good to you? If so I can merge it when I'm done checking)

MaximeBouton · 2019-10-09T21:48:50Z

I found a couple of DDNNode remaining, I am on it.

A second point is that some of these implementation could really be modernized (especially TMaze), I am not sure if we should do it now or in a separate PR.

zsunberg · 2019-10-09T21:58:06Z

I found a couple of DDNNode remaining, I am on it.

Haha, wait @MaximeBouton DDNNode itself is not always wrong! It is only that specific one for the initial observation.

A second point is that some of these implementation could really be modernized (especially TMaze), I am not sure if we should do it now or in a separate PR.

I vote another PR. Usually better to just get everything working as soon as possible, then make it nicer later

zsunberg · 2019-10-09T22:00:18Z

@MaximeBouton the only bad DDNNode I see is the one in Tiger. The one in Mountain Car is right.

MaximeBouton · 2019-10-09T22:00:33Z

The tiger POMDP had this:

function gen(::DDNNode{:o}, p::TigerPOMDP, s::Bool, rng::AbstractRNG)
    d = observation(p, 0, s) # obs distribution not action dependent
    return rand(rng, d)
end

This should typically be replaced by initialobs.
I am fine with waiting for POMDPs v0.8.1

Shushman · 2019-10-09T23:38:11Z

Thanks for helping see this to completion!

MaximeBouton · 2019-10-10T20:38:17Z

@zsunberg did I get the compat right?

zsunberg · 2019-10-11T01:12:21Z

@zsunberg did I get the compat right?

I think it should just be "0.8.1". https://julialang.github.io/Pkg.jl/v1/compatibility/index.html#Caret-specifiers-1 - no specifier is the same as a caret specifier, and it says "^0.2.3" is compatible with the range [0.2.3, 0.3.0), and the range we want is [0.8.1, 0.9.0). I made the change. I think it looks good now though! Tag a release when ready or I can if you want me to.

Shushman added 2 commits September 26, 2019 14:17

Julia 1.0 tests pass

4129be5

Only using DDNNode where needed

773fade

Shushman requested a review from MaximeBouton September 26, 2019 21:35

MaximeBouton requested changes Sep 27, 2019

View reviewed changes

MaximeBouton added 2 commits September 26, 2019 17:07

no email in travis

4fcb4d0

travis windows + julia version

a053582

zsunberg reviewed Sep 27, 2019

View reviewed changes

src/CryingBabies.jl Outdated Show resolved Hide resolved

zsunberg reviewed Sep 27, 2019

View reviewed changes

Project.toml Outdated Show resolved Hide resolved

zsunberg reviewed Sep 27, 2019

View reviewed changes

src/TigerPOMDPs.jl Outdated Show resolved Hide resolved

zsunberg reviewed Sep 27, 2019

View reviewed changes

src/TigerPOMDPs.jl Outdated Show resolved Hide resolved

Shushman added 3 commits September 27, 2019 10:28

Addressed Zach's PR comments except for observation

61aeab3

Merge branch 'pomdps-v0.8-compat' of https://github.com/JuliaPOMDP/PO…

7aee1a3

…MDPModels.jl into pomdps-v0.8-compat

Changed to DDNNode for initial observation

6c743a2

MaximeBouton added 2 commits October 8, 2019 15:26

fix TMaze

ceb4437

remove gw deprecation, remove tikzpictures

9b2fa48

MaximeBouton approved these changes Oct 9, 2019

View reviewed changes

support only POMDPs 0.8

54523de

Remove some extraneous stuff from baby problem

3ae58bc

MaximeBouton added 2 commits October 10, 2019 13:32

initialobs tiger

4f6972c

compat pomdps >= 0.8.1

c5ea4da

MaximeBouton and others added 2 commits October 10, 2019 14:41

import initialobs, fix tests

fde04d9

compat to 0.8.1

4d5dc46

MaximeBouton merged commit 608dd9c into master Oct 11, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compatibility with the POMDPs v0.8 release #56

Compatibility with the POMDPs v0.8 release #56

Shushman commented Sep 26, 2019

coveralls commented Sep 26, 2019 •

edited

MaximeBouton left a comment

zsunberg commented Sep 27, 2019

zsunberg commented Sep 27, 2019

zsunberg Sep 27, 2019

Shushman Sep 27, 2019

zsunberg Sep 27, 2019

Shushman Sep 27, 2019

zsunberg Sep 27, 2019

Shushman Sep 27, 2019

Shushman commented Sep 27, 2019

MaximeBouton commented Oct 9, 2019

zsunberg commented Oct 9, 2019

MaximeBouton commented Oct 9, 2019

zsunberg commented Oct 9, 2019

MaximeBouton commented Oct 9, 2019

zsunberg commented Oct 9, 2019

zsunberg commented Oct 9, 2019

MaximeBouton commented Oct 9, 2019

Shushman commented Oct 9, 2019

MaximeBouton commented Oct 10, 2019

zsunberg commented Oct 11, 2019

Compatibility with the POMDPs v0.8 release #56

Compatibility with the POMDPs v0.8 release #56

Conversation

Shushman commented Sep 26, 2019

coveralls commented Sep 26, 2019 • edited

MaximeBouton left a comment

Choose a reason for hiding this comment

zsunberg commented Sep 27, 2019

zsunberg commented Sep 27, 2019

zsunberg Sep 27, 2019

Choose a reason for hiding this comment

Shushman Sep 27, 2019

Choose a reason for hiding this comment

zsunberg Sep 27, 2019

Choose a reason for hiding this comment

Shushman Sep 27, 2019

Choose a reason for hiding this comment

zsunberg Sep 27, 2019

Choose a reason for hiding this comment

Shushman Sep 27, 2019

Choose a reason for hiding this comment

Shushman commented Sep 27, 2019

MaximeBouton commented Oct 9, 2019

zsunberg commented Oct 9, 2019

MaximeBouton commented Oct 9, 2019

zsunberg commented Oct 9, 2019

MaximeBouton commented Oct 9, 2019

zsunberg commented Oct 9, 2019

zsunberg commented Oct 9, 2019

MaximeBouton commented Oct 9, 2019

Shushman commented Oct 9, 2019

MaximeBouton commented Oct 10, 2019

zsunberg commented Oct 11, 2019

coveralls commented Sep 26, 2019 •

edited