Ticket #13018 (closed enhancement: fixed)

Opened 12 months ago

Last modified 11 months ago

Positive definite check for exact matrices

Reported by: rbeezer Owned by: jason, was
Priority: minor Milestone: sage-5.1
Component: linear algebra Keywords: sd40.5
Cc: novoselt Work issues:
Report Upstream: N/A Reviewers: Dan Drake
Authors: Rob Beezer Merged in: sage-5.1.beta4
Dependencies: #12966 Stopgaps:

Description (last modified by jdemeyer) (diff)

Implements an "is_positive_definite()" method for matrices over exact rings.

Apply:

  1. trac_13018-is-positive-definite-v2.patch Download

Attachments

trac_13018-is-positive-definite-v2.patch Download (9.0 KB) - added by rbeezer 12 months ago.

Change History

comment:1 Changed 12 months ago by rbeezer

  • Cc novoselt added
  • Status changed from new to needs_review
  • Dependencies set to #12966
  • Description modified (diff)
  • Authors set to Rob beezer

comment:2 Changed 12 months ago by rbeezer

  • Keywords sd40.5 added

comment:3 Changed 12 months ago by ddrake

  • Reviewers set to Dan Drake
  • Authors changed from Rob beezer to Rob Beezer

This looks good. Several comments:

  • Minor grammar error: "This routine will return True if the matrix is square, symmetric or Hermitian, and meeting the condition..." should be "and meets the condition...".
  • for the matrices that aren't positive definite, maybe you can include a doctest that has a vector for which v^T * M * v is negative.
  • for the matrices that are pos. def., maybe create a random vector in the base ring and see if you get something positive. I'm thinking: 
    sage: v = vector([C.random_element(), C.random_element(), C.random_element(), C.random_element()])
sage: v.conjugate() * A * v > 0
True

Changed 12 months ago by rbeezer

comment:4 Changed 12 months ago by rbeezer

  • Description modified (diff)

Thanks, Dan. Standalone "v2" patch fixes the grammar. For matrices that are positive definite the doctest now includes the (positive) determinants of the leading principal submatrices. For those that are not positive definite there is an example vector violating the defining condition (which was a great suggestion).

comment:5 Changed 12 months ago by novoselt

Apply only trac_13018-is-positive-definite-v2.patch

comment:6 Changed 12 months ago by novoselt

Somehow this ticket is already marked as "positive review" on the wiki page. Dan - do you agree with the verdict?

comment:7 Changed 12 months ago by ddrake

  • Status changed from needs_review to positive_review

Yes. I was just trying to set it to positive review. :)

comment:8 Changed 12 months ago by jdemeyer

  • Description modified (diff)

comment:9 Changed 11 months ago by jdemeyer

  • Status changed from positive_review to closed
  • Resolution set to fixed
  • Merged in set to sage-5.1.beta4
Note: See TracTickets for help on using tickets.