Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#11104 closed enhancement (fixed)

Add check for normal matrices

Reported by: rbeezer Owned by: jason, was
Priority: minor Milestone: sage-5.0
Component: linear algebra Keywords:
Cc: jason Merged in: sage-5.0.beta9
Authors: Rob Beezer Reviewers: David Loeffler
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11027, #10848 Stopgaps:

Description (last modified by rbeezer)

An exact method that compares entries of the two products and an RDF/CDF version that has both a naive check like the exact case and a more robust check based on the Schur decomposition.

Depends on #11027, #10848

Apply

  1. trac_11104-normal-matrices.patch
  2. trac_11104-docfix.patch

Attachments (2)

trac_11104-normal-matrices.patch (12.9 KB) - added by rbeezer 9 years ago.
trac_11104-docfix.patch (924 bytes) - added by davidloeffler 8 years ago.

Download all attachments as: .zip

Change History (17)

Changed 9 years ago by rbeezer

comment:1 Changed 9 years ago by rbeezer

  • Authors set to Rob Beezer
  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 8 years ago by chapoton

  • Dependencies set to #11027, #10848

comment:3 Changed 8 years ago by jason

  • Cc jason added

comment:4 Changed 8 years ago by davidloeffler

  • Reviewers set to David Loeffler
  • Status changed from needs_review to positive_review

Looks good, and all doctests pass

Changed 8 years ago by davidloeffler

comment:5 Changed 8 years ago by davidloeffler

  • Status changed from positive_review to needs_work

comment:6 Changed 8 years ago by davidloeffler

  • Status changed from needs_work to needs_review

Jeroen pointed out at #11306 two warnings building the documentation that actually come from this ticket. Here's a patch that fixes them.

comment:7 Changed 8 years ago by rbeezer

David - you are far too fast for me. I was going to try to clean up my mess. So I'll review the fix instead.

On sage-5.0.beta5 these patches both apply and the tests in sage/matrix all pass. Documentation builds without errors and the is_normal() section of matrix_double_dense.html looks fine.

I'll run full tests in a minute and then flip to positive review.

comment:8 Changed 8 years ago by rbeezer

  • Description modified (diff)

comment:9 Changed 8 years ago by rbeezer

  • Status changed from needs_review to positive_review

Positive review. I'm inclined to just leave the author/reviewer fields as-is, but if David wants to double them up, that's fine too.

Thanks for the review, David.

comment:10 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.0.beta9
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:11 follow-up: Changed 8 years ago by jhpalmieri

Using the sage-5.0.beta10-gcc tarball from #12369, I get a doctest failure on OS X Lion:

sage -t  --long -force_lib devel/sage/sage/matrix/matrix_double_dense.pyx
**********************************************************************
File "/Users/palmieri/Desktop/Sage_stuff/sage_builds/GCC-no-check/sage-5.0.beta10-gcc/devel/sage-main/sage/matrix/matrix_double_dense.pyx", line 2843:
    sage: B.is_normal(algorithm='naive', tol=1.0e-34)
Expected:
    False
Got:
    True
**********************************************************************

If I run this by hand, then even

    sage: B.is_normal(algorithm='naive', tol=1.0e-300)

returns True. The matrix B:

sage: B
[                                    1.0 3.46944695195e-16 + 3.88578058619e-16*I 2.22044604925e-16 + 2.77555756156e-17*I]
[3.46944695195e-16 - 3.88578058619e-16*I                                     1.0 6.93889390391e-17 - 1.11022302463e-16*I]
[2.22044604925e-16 - 2.77555756156e-17*I 6.93889390391e-17 + 1.11022302463e-16*I                                     1.0]

If I run B.is_normal(algorithm='orthonormal', tol=1.0e-16), I get False. With tol=1.0e-15, I get True.

Last edited 8 years ago by jhpalmieri (previous) (diff)

comment:12 in reply to: ↑ 11 Changed 8 years ago by rbeezer

Replying to jhpalmieri:

Thanks, John. Maybe this test is just too clever. ;-) It is trying to get a "wrong" answer by using a tolerance that is too strict. With what I know now about variability across OS'es, I would probably not attempt such a thing again. We could,

(a) Back it all out and make a fix, or
(b) Make a corrective ticket.

A fix could be

(i) Ditch the test, or
(ii) Keep it in spirit, but switch to the orthonormal version.

Any thoughts on the best way to proceed?

Rob

comment:13 follow-ups: Changed 8 years ago by jhpalmieri

Hi Rob,

I prefer options (b) (definitely: no need to back this out for a failure on one platform) and (i) (not as definitely: do you think having a test like this is important? I can't tell...). If you want option (ii), it might be annoying to make sure it passes on all of the different supported architectures.

comment:14 in reply to: ↑ 13 Changed 8 years ago by rbeezer

Replying to jhpalmieri:

Thanks, John.

I like (b)(i) as well. I don't think the test adds much in the way of "software assurance" and was meant to be "educational" for the newbie. So it shouldn't be missed. I'll see if I can get to it tonight.

Rob

comment:15 in reply to: ↑ 13 Changed 8 years ago by rbeezer

Replying to jhpalmieri:

Patch up shortly on #12764.

Note: See TracTickets for help on using tickets.