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

use deterministic RNG initial state #304

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

stevengj
Copy link
Member

@stevengj stevengj commented Oct 30, 2021

Closes #303.

Uses a deterministic RNG of MersenneTwister(seed) for generating initial vectors in iterative algorithms that use random initial vectors, controllable via an rng keyword.

(In some cases, the rng keyword is redundant, because the algorithm alternatively allows you to pass the initial vector directly, but it seemed better to me to have a consistent API that can be used with any of the iterative solvers doing pseudorandom initialization.)

(Bumped Julia requirement to 1.4 to support broadcasting RNG objects.)

@stevengj
Copy link
Member Author

Weird, I can't replicate the test failure locally…

@stevengj
Copy link
Member Author

stevengj commented Oct 30, 2021

Okay, now I can replicate a failure. It looks like one issue was that lobpcg! calls itself recursively, but I wasn't passing through the rng keyword, so it was generating the same pseudorandom numbers on each recursive call. But there is still some other failure I need to track down...

src/svdl.jl Show resolved Hide resolved
@mschauer
Copy link
Member

mschauer commented Nov 1, 2021

The commit a4db20b had some build's passing, are the errors just from having different seeds and too tights bounds for stochastic tests?

@stevengj
Copy link
Member Author

stevengj commented Nov 1, 2021

are the errors just from having different seeds and to tights bounds for stochastic tests?

Looks like it. I switched to master and changed the seed! line in test/lobpcg.jl to Random.seed!(Random.make_seed()), to ensure a different seed on every run, and now I'm getting the same failures about 25% of the time — I include that file 100 times, and it failed 26 times.

Note that one of the failures is not just a tolerance thing, but a robustness problem: the Cholesky factorization here is sometimes failing because the matrix is indefinite (a nearly zero eigenvalue that is slightly indefinite … basically it looks like X is rank-deficient, in which case you should probably do some re-initialization?).

The other failure is on

test/lobpcg.jl:316
  Expression: all(isapprox.(adjoint(X) * (B * X), Matrix{T}(I, 3, 3), atol = 2 * n * tol))

where it looks like the tolerance might be too small.

(Not sure why you are using all(isapprox.(..)) instead of just isapprox on the whole matrix here?)

Project.toml Outdated Show resolved Hide resolved
@mschauer
Copy link
Member

Ping @haampie

@mschauer
Copy link
Member

@stevengj What to do with the tests? I agree that we can soften a bound for the second failure.

@mschauer
Copy link
Member

mschauer commented Sep 1, 2022

There is one test passing,

CI / Julia 1.6 - ubuntu-latest - x64 - pull_request (pull_request)
which seems to be that the lobpg issue is fixed on some level. But shouldn't it pass on all systems as it is deterministic?

@mschauer mschauer closed this Aug 3, 2023
@mschauer mschauer reopened this Aug 3, 2023
@mschauer mschauer self-requested a review August 26, 2023 09:07
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.

accept user-specified rng for random-number generation
4 participants