-
Notifications
You must be signed in to change notification settings - Fork 26
Align2D refactor/updates #538
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
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #538 +/- ##
===========================================
- Coverage 87.20% 86.86% -0.34%
===========================================
Files 108 108
Lines 8000 8586 +586
===========================================
+ Hits 6976 7458 +482
- Misses 1024 1128 +104
Continue to review full report at Codecov.
|
this better fits future codes like EM
6c65173 to
2cffd1a
Compare
d7eb7f5 to
2b2ea9d
Compare
2b2ea9d to
d7df6e3
Compare
|
Todays commits meld the simulated experimental pipeline and associated minor patches into this PR. It alters the sphinx gallery config to only run the tutorials, but parse the experiments. I also extend the RIR and aligner to admit CWF denoised source for just the classification. Switching I'll try to go through it on another day to look for any typos I missed so far. |
|
Made some style and name changes to the example. Adds an experimental data pipeline and it even runs. Ran (on my laptop) for a small pipeline similar in size to the simulation example... but the reconstruction left a lot to be desired. I'll run it again in a more serious size while the code gets a once over. |
janden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass. Overall, I think this looks good. Big question here is about the class hierarchy.
I haven't gone into implementation details and tests yet. We'll get to that later.
5792514 to
478dff4
Compare
…ctor_Reddy_20220207
janden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Just some small comments, then we should be good to go.
| estimator = MeanEstimator(avgs, basis) | ||
|
|
||
| # Perform the estimation and save the volume. | ||
| estimated_volume = estimator.estimate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be able to compute an RMSE with the ground truth volume by rotating estimated_volume by Q_mat (Q_mat.T?) and comparing them. Or you might not. It's worth a shot. Otherwise, just leave it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can try it. I'll come back to this after the other items. I didn't have any idea that Q_mat would realize the right rotations for that... I'm not sure the project is there yet (good rmse), lots of things to check, but I was reasonably happy to get things like the attached at this stage of the project. First is the original hi res map from EMD 10028, and second is the simulated abinitio model, both as viewed in chimera.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW that sim was gallery/experiments/simulated_abinitio_pipeline.py, if you'd like to run it too. Just requires downloading the map file and putting in the directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That doesn't look too bad, actually…
So the idea is that Q_mat tells us how the estimated rotations differ from the ground truth rotations (in other words, what rotation needs to be applied to transform one rotation sequence to the other). Thing that might be tricky is that it may include a possible reflection as well. It's definitely a “nice to have” and not a requirement. If it takes more than fifteen minutes, make an issue and we'll come back to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was quite pleased with it for the maturity of the project and sort of rushing this last feature given the other situations. Lot of code refinement todo, but I think this should get the project down the road quite a ways.
I'll load the saved volume and give it a try.
|
Just waiting for the local docs and tests to run. I will push it for another round. While that is happening I will i) kick off |
|
Looks like some of the other changes that went in recently have changed the noise level we can preprocess away, so I'll need to update that. (One param line change in simulated experiment, I just need to try a few to see one that is good example). |
|
Simulated abinitio reconstruction looks similar now with the noise var adjustment. Not quite as good as before, but the whitening was working too well before :). Merging. |


Align2Dtowards subclassing. One branch going off the base class and returning some latent images (say for EM methods) and others which will do basic image stacking via subclassingAveragedAlign2D.radius.