ComputationalCryoEM / ASPIRE-Python

Algorithms for Single Particle Reconstruction
http://spr.math.princeton.edu
GNU General Public License v3.0
49 stars 21 forks source link

Check FFB3D Zeros #1148

Open garrettwrong opened 5 months ago

garrettwrong commented 5 months ago

One of our researchers sent an email describing some confusion between our code/comments and his expectations.

There seems to be a transpose typo somewhere in Aspire v0.11.1 when creating FFBBasis3D.
I am not sure if this has been corrected since that version, so I just wanted to run it by you
before submitting an issue.

MWE:

import numpy as np
from aspire.basis import *
M = 32
L = 8
py_basis = FFBBasis3D((M,M,M), ell_max = L, dtype=np.float32)

py_basis.r0

The last line displays ragged lists that are the transpose of what it should be, in order for
py_basis.r0[0] to be the list of zeros corresponding to j_0, i.e., to access what should be
py_basis.r0[0], I need to run

[py_basis.r0[i][0] for i in range(len(py_basis.r0))]

which outputs

[3.1415927, 6.2831855, 9.424778, 12.566371, 15.707963, 18.849556, 21.991148, 25.132742,
28.274334, 31.415926, 34.557518, 37.699112, 40.840706, 43.982296, 47.12389]

(which in particular has length py_basis.k_max[0])

Sorry if I am misinterpreting the syntax somewhere and if this has already been fixed in
the latest aspire versions, but it seems from lines 49-51 in

    https://github.com/ComputationalCryoEM/ASPIRE-Python/blob/main/src/aspire/basis/fb.py

that this should be the interpretation.

So far I tracked it back to this commit removing several m_ methods.

https://github.com/ComputationalCryoEM/ASPIRE-Python/commit/cb39a8af17d4524e7127dcab35245e8e077b9fdd

EDIT: Line wrapping.

garrettwrong commented 2 weeks ago

Let's visit this after the m_* PR and see if it is more straightforward. Currently I think the comment was incoherent with the code.