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

eigenmatrix_right returns inconsistent results for eigenvectors #8710

Closed
rbeezer mannequin opened this issue Apr 18, 2010 · 10 comments
Closed

eigenmatrix_right returns inconsistent results for eigenvectors #8710

rbeezer mannequin opened this issue Apr 18, 2010 · 10 comments

Comments

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Apr 18, 2010

Doctests introduced in #4756 return the negative of certain eigenvectors on certain hardware.

See initital discussion at

http://groups.google.com/group/sage-release/browse_thread/thread/9136569bd1c67f6

CC: @jhpalmieri @aghitza

Component: linear algebra

Author: Rob Beezer

Reviewer: William Stein, John Palmieri

Merged: sage-4.4.alpha1

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

@rbeezer rbeezer mannequin added this to the sage-4.4.1 milestone Apr 18, 2010
@rbeezer rbeezer mannequin assigned jasongrout and williamstein Apr 18, 2010
@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Apr 19, 2010

Attachment: trac_8710-eigenvector-doctest.patch.gz

@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Apr 19, 2010

comment:2

This patch massages the doctests that were causing failures for 4.4.alpha0 on the Skynet machine, sextus. Its not pretty, but I hope the results are now hardware-neutral.

Alex - you reviewed the original ticket (#4756), so maybe this would be an easy review for you?

John - I don't know if it is easy for you to test this on sextus in advance of merging it? Sounds like it will be a while before I have that kind of access.

Rob

@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Apr 19, 2010

Author: Rob Beezer

@rbeezer rbeezer mannequin added the s: needs review label Apr 19, 2010
@williamstein
Copy link
Contributor

comment:3
  1. John will have to test, since he has the build on sextus.

  2. The doctests actually look much nicer normalized to have first entry 1!

(I would give this a positive review if this works.)

@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Apr 19, 2010

comment:4

Replying to @williamstein:

  1. The doctests actually look much nicer normalized to have first entry 1!

The output looks nicer because this matrix is out of my textbook and was designed to have nice-looking answers. I don't like the hard-to-decipher code that gets you there, but that's the way it goes. Thanks for having a look and for the advice on getting this squared away.

Rob

@jhpalmieri
Copy link
Member

Reviewer: William Stein, John Palmieri

@jhpalmieri
Copy link
Member

comment:5

Tests pass on sextus. I'll test it on a few more machines, and if it works, I'll give it a positive review and merge it into 4.4.alpha1.

@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Apr 27, 2010

comment:6

Hi John,

Did this eventually past muster on skynet, or does it need more testing?

I still haven't done the SciPy tests I'd like to do skynet yet, but hope to get to that soon.

Rob

@jhpalmieri
Copy link
Member

Merged: sage-4.4.alpha1

@jhpalmieri
Copy link
Member

comment:7

Sorry, Rob. This was actually merged in 4.4.alpha1 but I forgot to mark it as closed. (So it didn't get recorded in the release notes, either.)

@sagetrac-mvngu sagetrac-mvngu mannequin modified the milestones: sage-4.4.1, sage-4.4 Apr 27, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants