Opened 10 years ago

Last modified 10 years ago

#13018 closed enhancement

Positive definite check for exact matrices — at Version 8

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

Status badges

Description (last modified by jdemeyer)

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

Apply:

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

Change History (9)

comment:1 Changed 10 years ago by rbeezer

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

comment:2 Changed 10 years ago by rbeezer

  • Keywords sd40.5 added

comment:3 Changed 10 years ago by ddrake

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

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 10 years ago by rbeezer

comment:4 Changed 10 years 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 10 years ago by novoselt

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

comment:6 Changed 10 years 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 10 years 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 10 years ago by jdemeyer

  • Description modified (diff)
Note: See TracTickets for help on using tickets.