Skip to content

Conversation

@albheim
Copy link
Member

@albheim albheim commented May 6, 2021

Remake of JuliaReinforcementLearning/ReinforcementLearningEnvironments.jl#141

  • Rename StateOverriddenEnv to StateTransformedEnv
  • Add state_space_mapping to StateTransformedEnv
  • Remove closure creation with constructors
  • Update/add some docstrings

@albheim
Copy link
Member Author

albheim commented May 6, 2021

Seems like something weird was going on with my editor. Tried to search for StateOverriddenEnv in the repo and only got the one hit in atari.jl outside of the wrapper definitions. Thought it was weird that it was not used more but didn't think more about it.

@findmyway findmyway merged commit 1c15e28 into master May 6, 2021
@albheim
Copy link
Member Author

albheim commented May 6, 2021

Found that even though tests passed we had an error in the outputs. Is something configured incorrectly?

At lines 1910 and forward in the output for the tests we have

 Error: Exception while generating log record in module Main at /home/runner/work/ReinforcementLearning.jl/ReinforcementLearning.jl/src/ReinforcementLearningZoo/test/runtests.jl:46
│   exception =
│    BoundsError: attempt to access Tuple{TotalRewardPerEpisode} at index [2]
│    Stacktrace:
│      [1] getindex
│        @ ./tuple.jl:29 [inlined]
│      [2] getindex(hook::ComposedHook{Tuple{TotalRewardPerEpisode}}, inds::Int64)
│        @ ReinforcementLearningCore ~/work/ReinforcementLearning.jl/ReinforcementLearning.jl/src/ReinforcementLearningCore/src/core/hooks.jl:51
│      [3] macro expansion
│        @ ./logging.jl:341 [inlined]
│      [4] (::var"#1#3")(dir::String)
│        @ Main ~/work/ReinforcementLearning.jl/ReinforcementLearning.jl/src/ReinforcementLearningZoo/test/runtests.jl:46
│      [5] mktempdir(fn::var"#1#3", parent::String; prefix::String)
│        @ Base.Filesystem ./file.jl:729
│      [6] mktempdir(fn::Function, parent::String) (repeats 2 times)
│        @ Base.Filesystem ./file.jl:727
│      [7] macro expansion
│        @ ~/work/ReinforcementLearning.jl/ReinforcementLearning.jl/src/ReinforcementLearningZoo/test/runtests.jl:35 [inlined]
│      [8] macro expansion
│        @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
│      [9] macro expansion
│        @ ~/work/ReinforcementLearning.jl/ReinforcementLearning.jl/src/ReinforcementLearningZoo/test/runtests.jl:35 [inlined]
│     [10] macro expansion
│        @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
│     [11] top-level scope
│        @ ~/work/ReinforcementLearning.jl/ReinforcementLearning.jl/src/ReinforcementLearningZoo/test/runtests.jl:34
│     [12] include(fname::String)
│        @ Base.MainInclude ./client.jl:444
│     [13] top-level scope
│        @ none:6
│     [14] eval
│        @ ./boot.jl:360 [inlined]
│     [15] exec_options(opts::Base.JLOptions)
│        @ Base ./client.jl:261
│     [16] _start()
│        @ Base ./client.jl:485
└ @ Main ~/work/ReinforcementLearning.jl/ReinforcementLearning.jl/src/ReinforcementLearningZoo/test/runtests.jl:46
  approximate the quantile values. The testing environment is CartPoleEnv.  This experiment uses the REMDQNLearner method with three dense layers to

Having a look at it now.

@albheim albheim deleted the albheim/fix_wrappers branch May 6, 2021 13:58
@findmyway
Copy link
Member

Yeah, nice catch! It's a bug in

and is triggered here:

Found that even though tests passed we had an error in the outputs. Is something configured incorrectly?

Not sure what I missed. But I'm pretty sure something is wrong here:

- run: |
julia --color=yes -e '
using Pkg;
Pkg.develop(path="src/ReinforcementLearningBase")
Pkg.develop(path="src/ReinforcementLearningCore")
Pkg.develop(path="src/ReinforcementLearningEnvironments")
Pkg.develop(path="src/ReinforcementLearningZoo")
Pkg.develop(path=".")
Pkg.test("ReinforcementLearningBase")
Pkg.test("ReinforcementLearningCore")
Pkg.test("ReinforcementLearningEnvironments")
Pkg.test("ReinforcementLearningZoo")
Pkg.test("ReinforcementLearning")'

@findmyway
Copy link
Member

Well, maybe not.

https://github.com/JuliaReinforcementLearning/ReinforcementLearning.jl/runs/2517792411#step:7:2461

There's no error shown in the final report...

@albheim
Copy link
Member Author

albheim commented May 6, 2021

Well, maybe not.

https://github.com/JuliaReinforcementLearning/ReinforcementLearning.jl/runs/2517792411#step:7:2461

There's no error shown in the final report...

Yeah, if you open the Run julia ... thing and scroll down to line 1910 there is the same error for me.

@findmyway
Copy link
Member

I just did a quick test. It seems something is not working as expected of the @info:

For example:

@testset "test" begin
    a = error("abc")
    @info "testing..." a
end

This will result in a report with an error:

Test Summary: | Error  Total
test          |     1      1
ERROR: LoadError: Some tests did not pass: 0 passed, 0 failed, 1 errored, 0 broken.
in expression starting at /mnt/E4E0A9C0E0A998F6/github/test_flux/abc/test/runtests.jl:3
ERROR: Package abc errored during testing

While the following one will not (just the same with the one in our tests):

using Test

@testset "test" begin
    @info "testing..." a = error("abc")
end
     Testing Running tests...
┌ Error: Exception while generating log record in module Main at /mnt/E4E0A9C0E0A998F6/github/test_flux/abc/test/runtests.jl:4
│   exception =
│    abc
│    Stacktrace:
│      [1] error(s::String)
│        @ Base ./error.jl:33
│      [2] macro expansion
│        @ logging.jl:341 [inlined]
│      [3] macro expansion
│        @ /mnt/E4E0A9C0E0A998F6/github/test_flux/abc/test/runtests.jl:4 [inlined]
│      [4] macro expansion
│        @ /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
│      [5] top-level scope
│        @ /mnt/E4E0A9C0E0A998F6/github/test_flux/abc/test/runtests.jl:4
│      [6] include(fname::String)
│        @ Base.MainInclude ./client.jl:444
│      [7] top-level scope
│        @ none:6
│      [8] eval
│        @ ./boot.jl:360 [inlined]
│      [9] exec_options(opts::Base.JLOptions)
│        @ Base ./client.jl:261
│     [10] _start()
│        @ Base ./client.jl:485
└ @ Main /mnt/E4E0A9C0E0A998F6/github/test_flux/abc/test/runtests.jl:4
Test Summary: |
test          | No tests
     Testing abc tests passed 

@findmyway
Copy link
Member

https://github.com/JuliaLang/julia/blob/ae1aa16bc0ee608568d799e25620ba9580110f9b/base/logging.jl#L343-L346

The error message is caught and not rethrown here. Not sure why they did this though...

@albheim
Copy link
Member Author

albheim commented May 6, 2021

So if an error occur inside an @info(...) it is not reported by the test as an error, but the error is still printed. That feels like a weird behaviour, some bug in the Test package?

EDIT: Git distracted while writing so didn't see that you already posted the answer.

@albheim
Copy link
Member Author

albheim commented May 6, 2021

https://github.com/JuliaLang/julia/blob/ae1aa16bc0ee608568d799e25620ba9580110f9b/base/logging.jl#L343-L346

The error message is caught and not rethrown here. Not sure why they did this though...

Seems weird, probably check with them if that is intended.

@findmyway
Copy link
Member

I created one here: https://discourse.julialang.org/t/why-is-the-exception-not-rethrown-in-logging/60658

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants