Fixes for 1.0 #23

JobJob · 2018-09-14T15:50:40Z

Fairly minimal set of changes to get this running on 1.0, and still maintain compatibility with 0.6 for now.

Required for JuliaML/OpenAIGym.jl#10 to work.

iblislin · 2018-09-17T01:44:37Z

.travis.yml

- - julia -e 'Pkg.clone(pwd()); Pkg.build("Reinforce"); Pkg.test("Reinforce"; coverage=false)'
+# script:
+#  - if [[ -a .git/shallow ]]; then git fetch --unshallow; fi
+#  - julia -e 'Pkg.clone(pwd()); Pkg.build("Reinforce"); Pkg.test("Reinforce"; coverage=false)'


It's fine to just removing the script section.

iblislin · 2018-09-17T01:51:03Z

test/interface.jl


 function test_ep_iteration()
-  info("interface::iteration::maxsteps")
+  @info "interface::iteration::maxsteps"


I think this @info will break the 0.6 support.
(But I'm fine about just dropping 0.6 support.)
Which will you vote fro? dropping or not dropping?

I actually added using Compat: @info here in runtests.jl, so that 0.6 tests pass, but anyway, I think we should drop 0.6 for future changes. I doubt many people, if anyone, are relying on this to work for 0.6.

I doubt many people, if anyone, are relying on this to work for 0.6.

There is a tagged stable release for 0.6. So it's okay to drop 0.6 support now.

My OpenAIGym PRs also work on 0.6 btw

Oh actually, just checked and JuliaML/OpenAIGym.jl/pull/10 works, but the others don't on 0.6

So, you mean all the PRs are based on 1.0, right?

Yeah, but it's prob a minor change to get them working for 0.6, OTOH 0.6 is old news 😄

well, I will vote for dropping it.

iblislin · 2018-09-17T12:23:18Z

.travis.yml

  - linux
 julia:
  - 0.6
+  - 1.0


and add 0.7?

I guess if it works on 1.0 it'll work on 0.7 though?

JobJob · 2018-09-17T13:59:17Z

So what's the verdict you want me to do anything more here?

iblislin · 2018-09-17T14:01:01Z

I need some time to test it manually

iblislin · 2018-09-18T13:54:00Z

Got this on 0.7

┌ Warning: Deprecated syntax `multiple line breaks between doc string and object` at /home/iblis/.julia/dev/Reinforce/src/Reinforce.jl:74.
│ Use `at most one line break` instead.
└ @ ~/.julia/dev/Reinforce/src/Reinforce.jl:74

patch:

diff --git a/src/Reinforce.jl b/src/Reinforce.jl
index 9ac3681..91a47ad 100644
--- a/src/Reinforce.jl
+++ b/src/Reinforce.jl
@@ -1,6 +1,3 @@
-
-__precompile__(true)
-
 module Reinforce
 
 using Reexport
@@ -70,7 +67,6 @@ finished(env::AbstractEnvironment, s′) = false
 
 Return a list/set/description of valid actions from state `s′`.
 """
-# actions(env::AbstractEnvironment) = actions(env, state(env))
 function actions end
 
 # note for developers: you don't need to implement these if you have state/reward fields

iblislin · 2018-09-18T14:02:19Z

WARNING: Base.srand is deprecated: it has been moved to the standard library package `Random`.
Add `using Random` to your imports.                                       
  likely near /home/iblis/.julia/dev/Reinforce/test/env/mountain_car.jl:3

patch:

diff --git a/src/envs/mountain_car.jl b/src/envs/mountain_car.jl
index 030fece..803b121 100644
--- a/src/envs/mountain_car.jl
+++ b/src/envs/mountain_car.jl
@@ -6,6 +6,7 @@ module MountainCarEnv

 using Reinforce: AbstractEnvironment
 using LearnBase: DiscreteSet
+using Random: srand
 using RecipesBase
 using Distributions

iblislin · 2018-09-18T14:15:23Z

miner change:

diff --git a/src/episodes/iterators.jl b/src/episodes/iterators.jl
index fd906e7..7d8b194 100644
--- a/src/episodes/iterators.jl
+++ b/src/episodes/iterators.jl
@@ -56,8 +56,7 @@ end


 @static if VERSION >= v"0.7"
-  Base.iterate(ep::Episode)    = _next(ep::Episode, _start(ep))
-  Base.iterate(ep::Episode, i) = _next(ep::Episode, i)
+  Base.iterate(ep::Episode, i::Integer = _start(ep)) = _next(ep::Episode, i)
 else
   Base.start(ep::Episode)   = _start(ep)
   Base.next(ep::Episode, i) = _next(ep, i)

iblislin · 2018-09-18T14:17:01Z

travis stuffs

diff --git a/.travis.yml b/.travis.yml
index cebc7d0..7270a8e 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,10 +4,7 @@ os:
   - linux
 julia:
   - 0.6
+  - 0.7
   - 1.0
-  - nightly
-matrix:
-  allow_failures:
-    - julia: nightly
 notifications:
   email: false

iblislin · 2018-09-18T14:40:10Z

ah, Base.srand is renamed to Random.seed!
My previous patch is wrong.

I want to drop 0.6 support and just write this

diff --git a/src/envs/mountain_car.jl b/src/envs/mountain_car.jl
index 030fece..0f1f88e 100644
--- a/src/envs/mountain_car.jl
+++ b/src/envs/mountain_car.jl
@@ -6,6 +6,7 @@ module MountainCarEnv
 
 using Reinforce: AbstractEnvironment
 using LearnBase: DiscreteSet
+using Random: seed!
 using RecipesBase
 using Distributions
 
@@ -45,7 +46,7 @@ MountainCar(seed=-1) = MountainCar(MountainCarState(0.0, 0.0), 0.0, seed)
 
 function reset!(env::MountainCar)
   if env.seed >= 0
-    srand(env.seed)
+    seed!(env.seed)
     env.seed = -1
   end

iblislin · 2018-09-18T14:47:44Z

src/envs/mountain_car.jl

+else
+  seed! = srand
+end
+


Both 0.7 and 1.0 are Random.seed!.
Invoking Random.srand in 0.7 will pop the deprecation warning up.

oh, you're right, i don't know how I got the impression that it wasn't.

iblislin

Thanks for your contributions!

JobJob · 2018-09-18T15:10:36Z

Thank you for taking the time to review!

JobJob added 2 commits September 15, 2018 01:44

Fixes for 1.0

5855c06

Use the default travis test script

b099967

This was referenced Sep 14, 2018

Update for 1.0, drop Universe, add some tests, and env.total_reward JuliaML/OpenAIGym.jl#10

Merged

Speed part 1 JuliaML/OpenAIGym.jl#11

Merged

Speed part 2 JuliaML/OpenAIGym.jl#12

Merged

Speed part 3 JuliaML/OpenAIGym.jl#13

Open

iblislin reviewed Sep 17, 2018

View reviewed changes

JobJob added 2 commits September 17, 2018 15:40

Remove commented script in travis config

603edcc

Add 1.0 to testing matrix

9a08929

iblislin reviewed Sep 17, 2018

View reviewed changes

Test on 0.7, Remove precompile(true) & minor fixes

36c6455

srandyness

4e3fb09

iblislin reviewed Sep 18, 2018

View reviewed changes

0.7 has seed! after all

de36bfd

iblislin approved these changes Sep 18, 2018

View reviewed changes

iblislin merged commit 77e2b17 into JuliaML:master Sep 18, 2018

JobJob deleted the fixes-for-1.0 branch September 18, 2018 15:12

Fixes for 1.0 #23

Fixes for 1.0 #23

Uh oh!

Conversation

JobJob commented Sep 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JobJob Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JobJob Sep 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JobJob commented Sep 17, 2018

Uh oh!

iblislin commented Sep 17, 2018

Uh oh!

iblislin commented Sep 18, 2018

Uh oh!

iblislin commented Sep 18, 2018

Uh oh!

iblislin commented Sep 18, 2018

Uh oh!

iblislin commented Sep 18, 2018

Uh oh!

iblislin commented Sep 18, 2018

Uh oh!

iblislin Sep 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iblislin left a comment

Choose a reason for hiding this comment

Uh oh!

JobJob commented Sep 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JobJob Sep 17, 2018 •

edited

Loading

JobJob Sep 17, 2018 •

edited

Loading

iblislin Sep 18, 2018 •

edited

Loading