Basis size fixes
#598
Merged
Basis size fixes
#598
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## develop #598 +/- ##
===========================================
+ Coverage 86.48% 87.42% +0.93%
===========================================
Files 108 108
Lines 8273 8952 +679
===========================================
+ Hits 7155 7826 +671
- Misses 1118 1126 +8
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
j-c-c
reviewed
Mar 24, 2022
Collaborator
j-c-c
left a comment
j-c-c
left a comment
There was a problem hiding this comment.
Looks good. A couple small comments.
src/aspire/basis/dirac.py
Outdated
| to include in the basis (default np.full(sz, True)). | ||
| """ | ||
| if isinstance(sz, int): | ||
| sz = (sz, sz) |
Collaborator
There was a problem hiding this comment.
Every other basis uses size instead of sz. Should we change this for consistency?
| below the Nyquist frequency (default Inf). | ||
| """ | ||
|
|
||
| if isinstance(size, int): |
Collaborator
There was a problem hiding this comment.
Maybe to cover these in testing we should initialize with an integer size in each basis test.
garrettwrong
pushed a commit
that referenced
this pull request
Apr 15, 2022
* fix size checking bug * fb2d * fb3d and pswf2d * dirac * fpswf * polar * format * sz -> size * test init with int * docstring * typo * the same typo
chris-langfield
added a commit
that referenced
this pull request
Aug 11, 2022
* Fix `k` bug in BFSReddyChetterji * restrict corr to a disc * Change `self.L` and `self.n` to `self.src.L` and `self.src.n` for estimators (#550) * changed self.L and self.n to self.src.L and self.src.n for estimators * extraneous comma * Add `aspire.utils.coor_trans.crop_2d` and move `coor_trans` objects to second-level API access (#565) * utils.coor_trans methods accessible from second level * crop_2d method * crop2d test * clarify flooring behavior for centering and add apdding * tests finished * isort shenanigans * fix even to odd n->n+1 padding bug * slightly better comment * better behavior for dtype * remove mistaken characters * fixes after merge * last ensure removed * rect tests * padding tests and fill value test * better comments * even better comments * np.diag * crop_pad_2d * clarifying comment when padding * Logical rather than hardcoded downsample tests (#567) * downsample tests * cleanup and deleting test downsample from preprocess_pipeline * remove hardcoded ds / whiten test * flake8 * 3d volume downsample test * remove redundant test from tests/test_preprocess.py * remove unused import * remove numbered tests for uniformity * typo * skip one of the ds tests * use gaussian_3d to generate test volume * isort * checkCenterPoint * max_resolution -> L_ds * parametrize ds tests * Bessel zero finding cleanup (#596) * docstring for besselj_zeros * rest of docstrings * remove pdb import * second order differences comment * comments and docstring for num_besselj_zeros * formatting * tiny fixes; * docstring fixes * NumPy * NumPy again * sanity check * parentheses inside sentence * `Basis` size fixes (#598) * fix size checking bug * fb2d * fb3d and pswf2d * dirac * fpswf * polar * format * sz -> size * test init with int * docstring * typo * the same typo * Authors page grammar fix. * cmap=gray * Gallery verb tense. Change cmap to gray. * Update gallery/experiments/README.rst header language. * Encode size in 2D FB test * Add test for 2D FB Gaussian expansion * Add eval–expand test for 2D FB * Add adjoint test for 2D FB * Add test for isotropic and modulated in 2D FB * Add test for indices * Add test for specific FB 2D basis element * Make 2D FB tests FFB friendly Cast `Images` to arrays when needed. * Move tests not specific to FB into a mixin * Add mixin to FFB 2D tests * Remove hardcoding from complex conversion tests * First attempt at testElement for FFB2D * Remove hardcoded (F)FB2D tests * Parameterize tests * Pass dtype to gaussian_2d * Expand on comment regarding factor of 1/2 * Fix rtoc bug * Add shape checks To make sure we don't get the same inadvertent broadcasting bug again. * Loop over different indices in 2D (F)FB tests * Fix #604 (#607) * Bump version: 0.9.0 → 0.9.1 * Fourier-Bessel Mixin class (#599) Fourier Bessel Mixin Class: fb.py * initial changes to code and tests * FFB bases * dirac * tentative fixes to mean estimator tests * import * covar3d mat_evaluate fix * unnecessary check * comments * black * fb2d check evaluate * fb3d * ffb3d * fb3d comments * dirac * fb2d * ffb2d * importvolume * pswf * fpswf * class checking logic moved to Basis * asnumpy in evaluate_t fb 3d bases * evaluate_t fixes * f string * fixes * fb2d image check consistent with fb3d * import * ISinstance * an Image * fix Dirac basis evaluate_t * dirac basis comments * remove volume check * Revert "remove volume check" This reverts commit 86d920e. * repr for Volume class * right place to squeeze in estimate() * no pdb * adjusted comments * remove new space clutter * clarified kernel_convolve squeeze * some changes * FB evaluate_t inputs are always Images/Volumes, just take transpose of asnumpy * remove dtype warnings * no pdb * diract evaluate_t take images * move repetitive function tests up to basis_util * cleanups * no self.L pswf * mat_evaluate_t * remove dirac * remove dirac tests * pswf * int size test in _basis_util * polar2d * remove image assert * remove type checks * space * spaces * final format * fix self.size typo * comment in mat_evaluate_t * replace DiracBasis with PolarBasis2D in unit test Co-authored-by: Garrett Wright <[email protected]> Co-authored-by: Garrett Wright <[email protected]> Co-authored-by: Joakim Andén <[email protected]> Co-authored-by: Josh Carmichael <[email protected]> Co-authored-by: Josh Carmichael <[email protected]>
chris-langfield
added a commit
that referenced
this pull request
Sep 1, 2022
* Fix `k` bug in BFSReddyChetterji * restrict corr to a disc * Change `self.L` and `self.n` to `self.src.L` and `self.src.n` for estimators (#550) * changed self.L and self.n to self.src.L and self.src.n for estimators * extraneous comma * Add `aspire.utils.coor_trans.crop_2d` and move `coor_trans` objects to second-level API access (#565) * utils.coor_trans methods accessible from second level * crop_2d method * crop2d test * clarify flooring behavior for centering and add apdding * tests finished * isort shenanigans * fix even to odd n->n+1 padding bug * slightly better comment * better behavior for dtype * remove mistaken characters * fixes after merge * last ensure removed * rect tests * padding tests and fill value test * better comments * even better comments * np.diag * crop_pad_2d * clarifying comment when padding * Logical rather than hardcoded downsample tests (#567) * downsample tests * cleanup and deleting test downsample from preprocess_pipeline * remove hardcoded ds / whiten test * flake8 * 3d volume downsample test * remove redundant test from tests/test_preprocess.py * remove unused import * remove numbered tests for uniformity * typo * skip one of the ds tests * use gaussian_3d to generate test volume * isort * checkCenterPoint * max_resolution -> L_ds * parametrize ds tests * Bessel zero finding cleanup (#596) * docstring for besselj_zeros * rest of docstrings * remove pdb import * second order differences comment * comments and docstring for num_besselj_zeros * formatting * tiny fixes; * docstring fixes * NumPy * NumPy again * sanity check * parentheses inside sentence * `Basis` size fixes (#598) * fix size checking bug * fb2d * fb3d and pswf2d * dirac * fpswf * polar * format * sz -> size * test init with int * docstring * typo * the same typo * Authors page grammar fix. * cmap=gray * Gallery verb tense. Change cmap to gray. * Update gallery/experiments/README.rst header language. * Encode size in 2D FB test * Add test for 2D FB Gaussian expansion * Add eval–expand test for 2D FB * Add adjoint test for 2D FB * Add test for isotropic and modulated in 2D FB * Add test for indices * Add test for specific FB 2D basis element * Make 2D FB tests FFB friendly Cast `Images` to arrays when needed. * Move tests not specific to FB into a mixin * Add mixin to FFB 2D tests * Remove hardcoding from complex conversion tests * First attempt at testElement for FFB2D * Remove hardcoded (F)FB2D tests * Parameterize tests * Pass dtype to gaussian_2d * Expand on comment regarding factor of 1/2 * Fix rtoc bug * Add shape checks To make sure we don't get the same inadvertent broadcasting bug again. * Loop over different indices in 2D (F)FB tests * Fix #604 (#607) * Bump version: 0.9.0 → 0.9.1 * Fourier-Bessel Mixin class (#599) Fourier Bessel Mixin Class: fb.py * New downsample method (#606) * dropped in new ds method * adjust tests temporarily * some imports * change to centered_fft2 and allow gridpoint checks * format * remove debug show() * comment * simplify centered_fft2 and centered_ifft2 * dict of dicts * adjusted tests * Zenodo file with authors and affiliations (#615) * add zenodo file with just creators * name * add zenodo file to manifest * json errors * chris orcid * updated affiliations * update tox.ini to include json.tool check * josh orcid * author ordering * review changes * Remove offending sk import * Explicitly use svd_solver='full' for repro unit test * Fix PIL deprecation warning * sensible defaults * optional saving mrc * populate_unique_filters and read_ctf_file * body of populate_unique_filters * loop var * tests * work around ImageSource.set_metadata * update test * final test * rationalize sample CTF values * readme space * correct metadata test * write_all_star * remove write_one_star * method name * reading from Relion micrograph_ctf.star * format * import_ctf() instead of __init__ option * added tests and clarified import_ctf method * condense ctf args coordinate source * .values() instead of items() * add save image flags to increase test converage * docstrings * rest of docstrings * rename fun to populate_ctf * first pass refactor * refactor 2 * docstring * refactor 3 Co-authored-by: Garrett Wright <[email protected]> Co-authored-by: Garrett Wright <[email protected]> Co-authored-by: Joakim Andén <[email protected]> Co-authored-by: Josh Carmichael <[email protected]> Co-authored-by: Josh Carmichael <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
1. Bug in #593
There is a check in
Basis.expand(), but it was not working properly. The input image was resized in such a way that it is it always passed the check (as can be seen from theFiles changed).For instance, a 32x32 image passed to a 16x16 basis was resized to (4,16,16), and since the last two dimensions match 16x16, it was allowed.
Simply moving the check before the resizing means that images that do not have the same dimension as the basis will be caught before the resizing.
2. Allow scalar
sizein__init__for bases