Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unit tests and deprecations on Julia v0.6 #230

Merged
merged 17 commits into from
May 5, 2017
Merged

Conversation

rdeits
Copy link
Contributor

@rdeits rdeits commented May 3, 2017

This PR fixes all of the unit tests on Julia v0.6 (as of today's release-0.6 branch). It also fixes all of the outstanding deprecation warnings except for a few relating to produce() and consume() which I don't understand well enough to fix yet.

Notable changes:

  • Changed NullBlob <: Blob to NullBlob <: Blob{Void, 0}. Please let me know if that's not correct.
  • Lots of instances of abs(::Vector) changed to abs.(::Vector)
  • abs(rand(Int, N)) % Y + X changed to rand(X:(X + Y - 1), N) to avoid a deprecation and make the function easier to understand
  • Array(T, size...) changed to Array{T}(size...)
  • abstract Foo changed to abstract type Foo end (with a @compat annotation for 0.5)
  • Explicitly added types to some constants in the tests which use non-Float64 types
  • Changed produce() and consume() to use channels on v0.6. This is not something I have much experience with, but I think I got it right

This does not fix:

  • Non-Julia backends (I don't have the native or GPU backends installed, so I haven't worked on them)
  • anything not covered by the unit tests

@pluskid
Copy link
Owner

pluskid commented May 4, 2017

Thank you very much!! It seems some of the unit tests on Travis CI is breaking. Could you have a look?

@rdeits
Copy link
Contributor Author

rdeits commented May 4, 2017

Sure! My most recent commit should fix v0.5 support. The test errors on v0.6 will be fixed with: kmsquire/Logging.jl#54 and I've updated the .travis.yml temporarily to try out my branch of Logging.jl

@rdeits
Copy link
Contributor Author

rdeits commented May 4, 2017

Hmm, the Julia v0.5 tests are stalling out. I'm not sure what's going on there. On the other hand, the v0.6 tests are running much faster than before.

Could this be due to the randomness inside the test functions? Would you consider fixing the random number seed to get more consistent test behavior?

@rdeits
Copy link
Contributor Author

rdeits commented May 4, 2017

The Travis build on v0.5 is consistently getting stuck here:

-- Testing ChannelPooling(Mocha.Pooling.Max) on Mocha.CPUBackend{Float64}...
    > Setup (pool along dimension 1 for 6-D tensors)
    > Forward

perhaps it's using enough memory to force the Travis VM to swap and thrash?

@pluskid
Copy link
Owner

pluskid commented May 4, 2017

Yes Pooling with 6D inputs takes a bit longer to run. Let's wait a bit since the nightly test has already passed. It should be ready to be merged once the 0.5 test pasts!

@rdeits
Copy link
Contributor Author

rdeits commented May 4, 2017

How do you want to handle the changes to Logging.jl? I don't know how long it will take for that PR to be merged.

@rdeits
Copy link
Contributor Author

rdeits commented May 4, 2017

It might be possible to replacing the use of Logging.jl with the new built-in logging capabilities in Julia v0.6: JuliaLang/julia#16213

@pluskid
Copy link
Owner

pluskid commented May 4, 2017

Yes, it is a bit tricky. We can merge this PR first and then either try to push the Logging PR to be merged or as you mentioned, try to use the built-in logging capability. Do you know when is the planned release for Julia v0.6?

@pluskid pluskid merged commit d4a1774 into pluskid:master May 5, 2017
@rdeits
Copy link
Contributor Author

rdeits commented May 5, 2017

Soon, hopefully :)

By the way, one reason that the v0.5 tests are so slow could be the coverage tracking. @tkoolen and I have noticed that running tests with coverage=true is much faster on v0.6 than v0.5, especially on Travis.

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.

None yet

2 participants