Opened 10 years ago

Closed 10 years ago

#8018 closed defect (fixed)

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

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

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

Download all attachments as: .zip

Change History (4)

comment:1 Changed 10 years ago by rbeezer

  • Authors set to Rob Beezer
  • Status changed from new to needs_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 10 years ago by rbeezer

comment:2 Changed 10 years ago by AlexGhitza

  • Reviewers set to Alex Ghitza
  • Status changed from needs_review to positive_review

Looks good.

comment:3 Changed 10 years ago by jhpalmieri

  • Merged in set to sage-4.4.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

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

Note: See TracTickets for help on using tickets.