Opened 13 years ago

Closed 13 years ago

#8018 closed defect (fixed)

Eigenvalues sorted, but not eigenvectors, in modular/modform/numerical.py

Reported by: Rob Beezer Owned by: Craig Citro
Priority: major Milestone: sage-4.4
Component: modular forms Keywords:
Cc: William Stein Merged in: sage-4.4.alpha0
Authors: Rob Beezer Reviewers: Alex Ghitza
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

In sage/modular/modform/numerical.py, the last half of _eigenvectors looks for eigenvectors with eigenvalues having multiplicty 1. The eigenvalues get sorted for openers, but the eigenvectors in B don't follow along.

Print statements before and after the sort, and then running doctests on just this file, produces output like:

    Hecke: before sort [-283.0, 108.522012456, -92.2176402155, -90.3043722401, 142.0]
    Hecke: after sort [-283.0, -92.2176402155, -90.3043722401, 108.522012456, 142.0]

One fix would be to delete the sorting if the order of the eigenvectors is not important.

All the doctests in this module that call this code lack eigenvalues of multiplicity greater than 1, so maybe a new doctest could test this case.

Also, it appears the cached value returned differs from the return at the bottom of the function.

Attachments (1)

trac_8018-numerical-eigenforms.patch (11.2 KB) - added by Rob Beezer 13 years ago.

Download all attachments as: .zip

Change History (4)

comment:1 Changed 13 years ago by Rob Beezer

Authors: Rob Beezer
Status: newneeds_review

This patch depends on #4756 due to intermediate changes in some of the CDF eigenvector code, so apply that patch first. Since this patch computes the eigenvalues directly, it is not necessary to understand #4756.

Summary:

  1. Return value has changed to be the subset of eigenvectors with multiplicity one, rather than all the eigenvectors. First few lines (immediate return without recalculation) indicate this was the intent.
  1. The eigenvalues do not get sorted now, fixing the observed bug. An extra check of uniq will cause the loop to speed-up when the eigenvector has high multiplicity.
  1. Eigenvalues and eigenvectors are computed directly via SciPy. This avoids various conversion overhead.
  1. Lots of blank lines marked as changed in the patch file. An accident of a massive cut/paste job to recover from an error.

Changed 13 years ago by Rob Beezer

comment:2 Changed 13 years ago by Alex Ghitza

Reviewers: Alex Ghitza
Status: needs_reviewpositive_review

Looks good.

comment:3 Changed 13 years ago by John Palmieri

Merged in: sage-4.4.alpha0
Resolution: fixed
Status: positive_reviewclosed

Merged "trac_8018-numerical-eigenforms.patch" in 4.4.alpha0

Note: See TracTickets for help on using tickets.