Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rbeezer mannequin opened this issue Jan 21, 2010 · 7 comments
Closed

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

rbeezer mannequin opened this issue Jan 21, 2010 · 7 comments

Comments

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented Jan 21, 2010

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.

CC: @williamstein

Component: modular forms

Author: Rob Beezer

Reviewer: Alex Ghitza

Merged: sage-4.4.alpha0

Issue created by migration from https://trac.sagemath.org/ticket/8018

@rbeezer rbeezer mannequin added this to the sage-4.4 milestone Jan 21, 2010
@rbeezer rbeezer mannequin added c: modular forms labels Jan 21, 2010
@rbeezer rbeezer mannequin assigned craigcitro Jan 21, 2010
@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Jan 24, 2010

comment:1

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.

  2. 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.

  3. Eigenvalues and eigenvectors are computed directly via SciPy. This avoids various conversion overhead.

  4. Lots of blank lines marked as changed in the patch file. An accident of a massive cut/paste job to recover from an error.

@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Jan 24, 2010

Author: Rob Beezer

@rbeezer rbeezer mannequin added the s: needs review label Jan 24, 2010
@rbeezer
Copy link
Mannequin Author

rbeezer mannequin commented Jan 24, 2010

Attachment: trac_8018-numerical-eigenforms.patch.gz

@aghitza
Copy link

aghitza commented Apr 3, 2010

Reviewer: Alex Ghitza

@aghitza
Copy link

aghitza commented Apr 3, 2010

comment:2

Looks good.

@jhpalmieri
Copy link
Member

comment:3

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

@jhpalmieri
Copy link
Member

Merged: sage-4.4.alpha0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants