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

linalg/symmetric test failure when run with no inlining #11089

Closed
kshyatt opened this issue May 1, 2015 · 15 comments
Closed

linalg/symmetric test failure when run with no inlining #11089

kshyatt opened this issue May 1, 2015 · 15 comments
Assignees

Comments

@kshyatt
Copy link
Contributor

kshyatt commented May 1, 2015

When running
julia --code-coverage=all --inline=no

import(CoverageBase)
using Base.Test
CoverageBase.runtests(CoverageBase.testnames())

On OSX 10.10.3, and on the Coveralls builds on Ubuntu, the linalg/symmetric tests fail at:

ERROR: LoadError: lengths of abs((eigfact(Hermitian(asym),d[1] - 10 * eps(d[1]),d[2] + 10 * eps(d[2])))[:vectors]' * v[:,1:2]) and eye(eltya,2) do not match:
  abs((eigfact(Hermitian(asym),d[1] - 10 * eps(d[1]),d[2] + 10 * eps(d[2])))[:vectors]' * v[:,1:2]) (length 2) = [0.9999999999999997 1.4155343563970746e-15]
  eye(eltya,2) (length 4) = [1.0 0.0
 0.0 1.0]
 in error at error.jl:20
 in test_approx_eq at test.jl:119
 in test_approx_eq at test.jl:147
 in anonymous at no file:35
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
 in anonymous at /Users/kshyatt/.julia/v0.4/CoverageBase/src/CoverageBase.jl:38
 in cd at file.jl:20
 in runtests at /Users/kshyatt/.julia/v0.4/CoverageBase/src/CoverageBase.jl:35
 in eval at no file
while loading /Users/kshyatt/Projects/julia/test/linalg/symmetric.jl, in expression starting on line 17
@tkelman
Copy link
Contributor

tkelman commented May 1, 2015

Awesome, thanks. So I think we had seen this one before - #10672 (comment) #10784

Evidently when running in coverage mode, we start encountering the numerical problems again?

@staticfloat
Copy link
Member

Well, this problem is more severe than a numerical problem; the two vectors are of unequal lengths!

@tkelman
Copy link
Contributor

tkelman commented May 1, 2015

See the links, and https://travis-ci.org/JuliaLang/julia/jobs/57437534#L3839 - I think we should call this an eigfact API bug, since

has trouble localizing the first two eigenvalues, and finds only one eigenpair

resulting in different-length output depending on input data seems fishy. Changing the seed so we didn't see it as often was really only a band-aid, sorry for not suggesting a better solution at the time.

@andreasnoack
Copy link
Member

I don't think I agree. We support calculating eigenpairs in some interval and that gives variable length result because, in general, you cannot know how many pairs there are in an interval. Why is this a bug?

@staticfloat
Copy link
Member

Supporting variable length results is fine, but in a test, I think we should have a way to coerce the output to the same length so that we can do equality tests that take into account the floating-point noise floor. Either an option to eigfact that forces it to return extra zeros, or we just zero-pad it on the receiving end.

@tkelman
Copy link
Contributor

tkelman commented May 1, 2015

We support calculating eigenpairs in some interval and that gives variable length result because, in general, you cannot know how many pairs there are in an interval. Why is this a bug?

You're right, I misread what's being requested here. This is indeed hard to know statically. It is an issue in the way the test is constructed though, since it's proving to not be very robust.

@andreasnoack
Copy link
Member

We could make the test more robust, but I'm wondering why the result is different when inlining is turned off. The seed is set in the beginning. Why would the random numbers be different?

@tkelman
Copy link
Contributor

tkelman commented May 1, 2015

That also deserves investigation as it could easily be a separate bug.

@simonster
Copy link
Member

If anything relies on a reduction that uses @simd, it may give a less accurate result with inlining disabled, e.g.:

julia> sum([1/n for n = 1:50])
4.499205338329425

but without inlining:

julia> sum([1/n for n = 1:50])
Warning: could not attach metadata for @simd loop.
4.499205338329423

@kshyatt
Copy link
Contributor Author

kshyatt commented May 1, 2015

I checked the matrix asym in this test both with and without inlining and it's the same in both cases.

@andreasnoack
Copy link
Member

I'm not sure it is inlining because

Andreass-MacBook-Pro:julia-dev andreasnoack$ make test-linalg
    JULIA test/linalg
    From worker 6:       * linalg/lapack        in   9.80 seconds
    From worker 5:       * linalg4              in  10.81 seconds
    From worker 8:       * linalg/tridiag       in  13.86 seconds
    From worker 8:       * linalg/givens        in   3.63 seconds
    From worker 5:       * linalg/pinv          in  14.69 seconds
    From worker 6:       * linalg/diagonal      in  16.97 seconds
    From worker 9:       * linalg/bidiag        in  27.86 seconds
    From worker 4:       * linalg3              in  36.41 seconds
    From worker 6:       * linalg/symmetric     in   9.98 seconds
    From worker 9:       * linalg/arnoldi       in  16.57 seconds
    From worker 3:       * linalg2              in  45.79 seconds
    From worker 5:       * linalg/lu            in  25.46 seconds
    From worker 8:       * linalg/cholesky      in  36.36 seconds
    From worker 2:       * linalg1              in  63.82 seconds
    From worker 7:       * linalg/triangular    in  79.12 seconds
    SUCCESS
Andreass-MacBook-Pro:julia-dev andreasnoack$ julia-dev
               _
   _       _ _(_)_     |  A fresh approach to technical computing
  (_)     | (_) (_)    |  Documentation: http://docs.julialang.org
   _ _   _| |_  __ _   |  Type "help()" for help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 0.4.0-dev+4589 (2015-05-01 17:40 UTC)
 _/ |\__'_|_|_|\__'_|  |  Commit 9fea76a* (0 days old master)
|__/                   |  x86_64-apple-darwin14.3.0

julia> include("test/linalg/symmetric.jl")
ERROR: LoadError: lengths of abs((eigfact(Hermitian(asym),d[1] - 10 * eps(d[1]),d[2] + 10 * eps(d[2])))[:vectors]' * v[:,1:2]) and eye(eltya,2) do not match: 
  abs((eigfact(Hermitian(asym),d[1] - 10 * eps(d[1]),d[2] + 10 * eps(d[2])))[:vectors]' * v[:,1:2]) (length 2) = [0.9999999999999998 1.4972574545598496e-15]
  eye(eltya,2) (length 4) = [1.0 0.0
 0.0 1.0]
 in test_approx_eq at test.jl:119
 in test_approx_eq at test.jl:147
 in anonymous at no file:35
 in include at ./boot.jl:250
 in include_from_node1 at ./loading.jl:129
while loading /Users/andreasnoack/julia-dev/test/linalg/symmetric.jl, in expression starting on line 17

@simonster
Copy link
Member

@andreasnoack In that case, I suspect it's single-threaded OpenBLAS (when run through the harness) vs. multi-threaded OpenBLAS (when run at the REPL or through CoverageBase). I get the same results as you when running at the REPL, but if I run Base.disable_threaded_libs() first the tests pass.

@kshyatt
Copy link
Contributor Author

kshyatt commented May 1, 2015

So should the Coveralls script run Julia in single-threaded OpenBLAS mode?

@andreasnoack
Copy link
Member

I've pushed a change of the tests such that we can be fairly sure that the interval includes two eigenvalues. The old interval was too narrow to be robust to the numerical differences between different eigensolvers. It was chance that this different between single and multithreaded eigendecomposition.

@kshyatt No, I think multithreaded is better as it corresponds to how most users run Julia.

@kshyatt
Copy link
Contributor Author

kshyatt commented May 2, 2015

@andreasnoack Thanks for explaining!

mbauman pushed a commit to mbauman/julia that referenced this issue Jun 6, 2015
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

No branches or pull requests

5 participants