Fix eigenmatrix doctest to work across all platforms #11897

rbeezer · 2011-10-05T04:46:58Z

Doctest for RDF matrix fails on some platforms by returning the negatives of the more commonly returned eigenvectors.

See sage-release discussion:
http://groups.google.com/group/sage-release/msg/e885e53316c6880f

Apply

attachment: trac_11897-doctest-RDF-eigenmatrix.rebased.patch
attachment: trac_11897-fix_noisy_zeroes_in_eigenvalues.reviewer.patch
to the Sage library.

CC: @kcrisman @jdemeyer @nexttime

Component: linear algebra

Author: Rob Beezer

Reviewer: Karl-Dieter Crisman, Leif Leonhardy

Merged: sage-4.7.2.alpha4

Issue created by migration from https://trac.sagemath.org/ticket/11897

rbeezer · 2011-10-05T04:50:37Z

Attachment: trac_11897-doctest-RDF-eigenmatrix.patch.gz

rbeezer · 2011-10-05T04:56:27Z

comment:1

Patch marks failing doctests as "not tested"

Rationale: this was just meant to show how to get results for inexact matrices with real or complex entries. It is in the middle of a docstring for an exact routine. So this preserves the "doc" part and abandons the "test" part.

Patch repeats, and fixes, doctest in the TEST section of the eigenmatrix routines.

Rational: doctest has not been abandoned. Due to the need to adjust the sign of the eigenvectors, this is relegated to a test section.

This was built on an alpha3 prerelease, I trust it will be OK on a real alpha3 (which I am about to build right now).

rbeezer · 2011-10-05T04:56:27Z

Author: Rob Beezer

nexttime · 2011-10-05T15:50:30Z

comment:2

Replying to @rbeezer:

This was built on an alpha3 prerelease, I trust it will be OK on a real alpha3 (which I am about to build right now).

Two hunks do not apply because of (the late) attachment: ticket:11595:trac_11595-fix_noisy_zero_doctest_errors.reviewer.patch.

Otherwise looks fine, but perhaps Karl-Dieter should rerun the tests on his famous favorite machine.

nexttime · 2011-10-05T16:05:39Z

comment:3

Do you also open a follup-up ticket to #7852?

attachment: ticket:7852:trac_7852-adjust_noisy_zero_term_threshold_for_polys.reviewer.patch wasn't enough for his machine. (I later increased epsilon from 1.0e-15 to 2.5e-15 in one example to make bsd.math happy, but he gets 2.6645352591e-15.)

kcrisman · 2011-10-05T17:10:29Z

comment:4

Two hunks do not apply because of (the late) attachment: ticket:11595:trac_11595-fix_noisy_zero_doctest_errors.reviewer.patch.

Correct. Let me know when you have a new one. :(

Otherwise looks fine, but perhaps Karl-Dieter should rerun the tests on his famous favorite machine.

Now, now! No disparaging remarks. At least I'm not using Windows or FreeBSD! Those pose more drastic problems :)

rbeezer · 2011-10-05T18:54:32Z

comment:5

Replying to @nexttime:

Do you also open a follup-up ticket to #7852?

No, did not want to venture into polynomials - just took responsibility for matrices.

Forgot to actually start a build last night, so it will be maybe 12 hours at the soonest before I can build a proper patch (long story). But will do.

Rob

nexttime · 2011-10-05T20:53:09Z

Attachment: trac_11897-doctest-RDF-eigenmatrix.rebased.patch.gz

Rob's patch rebased on Sage 4.7.2.alpha3.

nexttime · 2011-10-05T21:00:09Z

Reviewer: Karl-Dieter Crisman, Leif Leonhardy

nexttime · 2011-10-05T21:00:09Z

comment:6

Sorry, I could easily have done that yesterday, but I was too tired to even look at the rejects.

The rebased patch also passes tests and the documentation builds fine, so positive review from my side.

Karl-Dieter (or Dasher / student?), please finalize!

kcrisman · 2011-10-06T01:12:23Z

comment:7

Karl-Dieter (or Dasher / student?), please finalize!

You don't even want to know how old this computer is, nor how many hands it's passed through on the way to me. But luckily the password is still rms' favorite - carriage return - and its only use is for testing Sage, so that's all I need! I know it's a burden sometimes, but I think that it's picked up a number of (real) bugs over the last couple years as well, vindicating Dave Kirkby's philosophy of "test as widely as possible".

Anyway, I'll get that fired up tomorrow morning when I get in.

Eventually I'll probably convert it (and a couple other similar machines I own) to Linux. Though now that YouTube has the HTML5 option, maybe I can start avoiding Flash and keep them going a few more years... see also TenFourFox. Favorite quote: "A quad 2.5GHz G5 isn't worth using to surf the web? Really? And you guys still support Windows XP?"

kcrisman · 2011-10-06T15:51:06Z

comment:8

Really sorry, folks.

Dasher-03:~/Desktop/sage-4.7.2.alpha3/devel/sage student$ ../../sage -t -long devel/sage/sage/matrix/matrix2.pyx
sage -t -long "devel/sage/sage/matrix/matrix2.pyx"          
**********************************************************************
File "/Users/student/Desktop/sage-4.7.2.alpha3/devel/sage/sage/matrix/matrix2.pyx", line 5282:
    sage: evalues = em[0]; evalues
Expected:
    [    13.3484692...                 0                 0]
    [                0    -1.34846922...                 0]
    [                0                 0 -6.2265089...e-16]
Got:
    [     13.3484692283                  0                  0]
    [                 0     -1.34846922835                  0]
    [                 0                  0 -1.35443935375e-15]
**********************************************************************
File "/Users/student/Desktop/sage-4.7.2.alpha3/devel/sage/sage/matrix/matrix2.pyx", line 5371:
    sage: evalues = em[0]; evalues
Expected:
    [     13.3484692...                  0                  0]
    [                 0     -1.34846922...                  0]
    [                 0                  0 -8.86256604...e-16]
Got:
    [    13.3484692283                 0                 0]
    [                0    -1.34846922835                 0]
    [                0                 0 1.74841524385e-16]
**********************************************************************
2 items had failures:
   1 of  21 in __main__.example_63
   1 of  21 in __main__.example_64
***Test Failed*** 2 failures.
For whitespace errors, see the file /Users/student/.sage//tmp/matrix2_27324.py
         [262.0 s]
 
----------------------------------------------------------------------
The following tests failed:


        sage -t -long "devel/sage/sage/matrix/matrix2.pyx"

We may need to use the new # tol construction for this one :(

nexttime · 2011-10-06T16:07:20Z

comment:9

Replying to @kcrisman:

We may need to use the new # tol construction for this one :(

Hmmm, would be nicer to expect a true zero (0) there and once again use .zero_at(2e-15) (or 2e-16 in one case)...

If you use # tol, that applies to all entries in the same way, i.e. with the same relative or absolute tolerance, which IMHO wouldn't be appropriate here.

nexttime · 2011-10-06T16:15:34Z

comment:10

Nope, .dense_matrix().zero_at(2e-15) in both cases, and expect 0 in the lower right.

nexttime · 2011-10-06T16:24:56Z

Sage library patch. Apply on top of Rob's (rebased) patch.

nexttime · 2011-10-06T16:28:11Z

comment:11

Attachment: trac_11897-fix_noisy_zeroes_in_eigenvalues.reviewer.patch.gz

Next try.

(I've attached a reviewer patch to be applied on top.)

Hopefully 2e-15 suffices for bsd.math as well... Apple c**p...

rbeezer · 2011-10-06T16:40:12Z

comment:13

Replying to @kcrisman:

Really sorry, folks.

No need to apologize - I was sort of afraid this would happen.

I have an alpha3 build now and will get to whatever needs doing (review of the latest patch?) tonight if it is still outstanding.

Rob

nexttime · 2011-10-06T16:56:18Z

comment:14

Replying to @nexttime:

Hopefully 2e-15 suffices for bsd.math as well...

Surprisingly it does. :)

kcrisman · 2011-10-06T19:10:32Z

comment:15

I agree that this solution is better than what I suggested. This passes the tests. Positive review?

sage -t -long "devel/sage/sage/matrix/matrix2.pyx"          
         [261.2 s]
 
----------------------------------------------------------------------
All tests passed!
Total time for all tests: 261.5 seconds

nexttime · 2011-10-06T19:37:03Z

comment:16

Replying to @rbeezer:

Replying to @nexttime:

Do you also open a follup-up ticket to #7852?

No, did not want to venture into polynomials - just took responsibility for matrices.

Patch for that is up on #11901, needing review... :P

nexttime · 2011-10-06T19:40:25Z

comment:17

Replying to @kcrisman:

I agree that this solution is better than what I suggested. This passes the tests. Positive review?

Why not... Although Rob could review my reviewer patch.

Anyway, he can set it back to needs work just in case... :)

rbeezer · 2011-10-07T03:49:30Z

comment:18

Replying to @nexttime:

Why not... Although Rob could review my reviewer patch.

Anyway, he can set it back to needs work just in case... :)

It all looks good to me, so I think we are done with this one. I'll go look at polynomials.

Thanks for the help with this one, Leif, and to KDC and Dasher for pushing us to do better. ;-)

jdemeyer · 2011-10-07T19:11:19Z

Merged: sage-4.7.2.alpha4

rbeezer mannequin added this to the sage-4.7.2 milestone Oct 5, 2011

rbeezer mannequin added c: linear algebra labels Oct 5, 2011

rbeezer mannequin assigned jasongrout and williamstein Oct 5, 2011

rbeezer mannequin added the s: needs review label Oct 5, 2011

nexttime mannequin added s: needs work and removed s: needs review labels Oct 5, 2011

This comment has been minimized.

Sign in to view

nexttime mannequin added s: needs review and removed s: needs work labels Oct 5, 2011

kcrisman added s: needs work and removed s: needs review labels Oct 6, 2011

This comment has been minimized.

Sign in to view

nexttime mannequin added s: needs review and removed s: needs work labels Oct 6, 2011

kcrisman added s: positive review and removed s: needs review labels Oct 6, 2011

jdemeyer removed the s: positive review label Oct 7, 2011

jdemeyer closed this as completed Oct 7, 2011

nexttime mannequin mentioned this issue Oct 8, 2011

Fix more noise on MacOS X 10.4 PPC #11901

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix eigenmatrix doctest to work across all platforms #11897

Fix eigenmatrix doctest to work across all platforms #11897

rbeezer mannequin commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

kcrisman commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

This comment has been minimized.

nexttime mannequin commented Oct 5, 2011

kcrisman commented Oct 6, 2011

kcrisman commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

This comment has been minimized.

nexttime mannequin commented Oct 6, 2011

rbeezer mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

kcrisman commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

rbeezer mannequin commented Oct 7, 2011

jdemeyer commented Oct 7, 2011

Fix eigenmatrix doctest to work across all platforms #11897

Fix eigenmatrix doctest to work across all platforms #11897

Comments

rbeezer mannequin commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

kcrisman commented Oct 5, 2011

rbeezer mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

nexttime mannequin commented Oct 5, 2011

This comment has been minimized.

nexttime mannequin commented Oct 5, 2011

kcrisman commented Oct 6, 2011

kcrisman commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

This comment has been minimized.

nexttime mannequin commented Oct 6, 2011

rbeezer mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

kcrisman commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

nexttime mannequin commented Oct 6, 2011

rbeezer mannequin commented Oct 7, 2011

jdemeyer commented Oct 7, 2011