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

Test using StableRNGs #55

Merged
merged 12 commits into from
Dec 10, 2020
Merged

Conversation

AsafManela
Copy link
Collaborator

Starting with Julia 1.5 random number generators are not guaranteed to provide the same sequence of numbers for a given seed (see here).

This PR replaced the global rng used in tests with a StableRNG called testrng and uses it everywhere.
Turns out that was not enough because Lasso.jl has a RandomCoefficientIterator that uses its own RNG.
So to get deterministic test, I also add the ability to specify an rng in the call to fit(...; rng=StableRNG(123)).

Tests now seem to pass on 1.5.
Closes #48

@codecov-io
Copy link

codecov-io commented Dec 9, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@81ec2a4). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #55   +/-   ##
=========================================
  Coverage          ?   92.45%           
=========================================
  Files             ?        8           
  Lines             ?     1113           
  Branches          ?        0           
=========================================
  Hits              ?     1029           
  Misses            ?       84           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81ec2a4...b4f5c21. Read the comment docs.

@AsafManela
Copy link
Collaborator Author

Hi @andreasnoack, do you have time to review this one or should I just merge?

@AsafManela AsafManela merged commit c028fd7 into JuliaStats:master Dec 10, 2020
@AsafManela AsafManela deleted the am/stablernd branch December 10, 2020 20:17
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.

possible test failure in upcoming Julia version 1.5
2 participants