estimate_ctf integration with CoordinateSource
#623
Merged
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
Merge pull request #584 from ComputationalCryoEM/develop
BFSReddyChetterji k Bug
…imators (#550) * changed self.L and self.n to self.src.L and self.src.n for estimators * extraneous comma
…o 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
* 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
* 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
* fix size checking bug * fb2d * fb3d and pswf2d * dirac * fpswf * polar * format * sz -> size * test init with int * docstring * typo * the same typo
Cast `Images` to arrays when needed.
j-c-c
reviewed
Jul 1, 2022
Collaborator
j-c-c
left a comment
j-c-c
left a comment
There was a problem hiding this comment.
Overall, looks great. I definitely get an involuntary twitch when i see the very similar lines in _populate_ctf_from_relion and _populate_ctf_from_list. Is there a way to package the Relion ctf info in the same form as ASPIRE's to pass into a single _populate_ctf() method? Or do you think that would be too confusing?
This was
linked to
issues
Jul 6, 2022
garrettwrong
requested changes
Jul 11, 2022
Collaborator
garrettwrong
left a comment
garrettwrong
left a comment
There was a problem hiding this comment.
Hey Chris, this is quite a bit of work, thank you for putting it together.
I put a couple very small things, and one actual design question in import_ctf (maybe I'm just missing detail there).
I didn't leave comments about it inline, but there might be a different way to do the test ranges for the unit tests. However, I think that would just be busy work. (so don't worry about it).
3 tasks
chris-langfield
force-pushed
the
ctf_estimator_result
branch
from
5454fee to
6456595
Compare
Collaborator
Author
janden
approved these changes
Sep 1, 2022
garrettwrong
approved these changes
Sep 1, 2022
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.
#622
Also addresses #633
Features added to
ctf_estimatorestimate_ctf()now returns a dictionary of dictionaries asresults. This is of the form:estimate_ctf()now has default CTF options, the same as in theestimate-ctfcommands, includingoutput_direstimate_ctf()now has the optionsave_ctf_images=False, which turns on or off saving the CTF itself to a.ctf(.mrc) file.estimate_ctf()now has the optionsave_noise_images=False, which turns on or off saving the noise as a.mrcFeatures added to
CoordinateSourceCoordinateSources now have theB=0option when initializing, which is required for the CTFCoordinateSources now have theimport_ctf()method, which uses new methods_populate_ctf_from_relion(),_populate_ctf_from_list()and utility_read_ctf_star(), which reads the input ctf files, construct theCTFFilters, and update thefilter_indicesand metadata.micrograph_ctf.starfile generated by RELION's wrapper of CTFFind for both v3.1 and v4.0 https://relion.readthedocs.io/en/latest/SPA_tutorial/Preprocessing.html#ctf-estimationhttps://relion.readthedocs.io/en/release-3.1/SPA_tutorial/Preprocessing.html#
Testing
test_CtfEstimatemodified to match new output ofestimate_ctf()test_coordinate_sourcenow has testing for the newimport_ctf()method, which also ensures that the source's filters and metadata are correct.