Implement power spectrum normalization and include it in SVD pipeline closes #14 #15
Conversation
|
Made a mistake when I pushed here, and pushed changes that were supposed to be for issue #10. It is a very small change, simply added that the population file has to be added explicitly in the preprocessing config. Changed the preprocessing tutorial accordingly |
I am not very familiar with NUFFT in Pytorch (I guess you are talking about this package?). If that is the case, probably this jupyter notebook is useful. It seems at the end of it an inverse transformation is performed. |
There was a problem hiding this comment.
It would be nice to add to the template svd_config.yaml file the new mandatory parameters (ref_vol_key, ref_vol_index, and output_file).
Also, having a short description in the SVD jupyter notebook of those new keys might be helpful for future users. Something like:
- ref_vol_key (str): the identifier of the submission where the reference volume needed to normalize the data will be taken (such as the icecream flavour of one of the submissions)
- ref_vol_index (int): in combination with ref_vol_key, determines the volume that will be set as the reference for the normalization of the data
- output_file (path): file where the ouputs will be saved. Must have .pt extension - (I think output_file substitutes output_path right?)
|
I think we can make the repo public before merging the PR. |
|
I implemented the changes. Also, I updated the config to make it more clear that the keys are related to power spectrum normalization. |
|
@DSilva27 can you get the unit tests working (can be merged in from main), so I can see they pass in the PR? |
Update .pre-commit-config.yaml
|
Merged from main (actually, from 39-fix-small-bug-in-preprocessing-pipeline, so revise that one first). This PR does the following updates Major Changes
Minor Changes
|
| do: true | ||
| volume: data/Ground_truth/mask_dilated_wide_224x224.mrc | ||
| volume: tests/data/Ground_truth/mask_dilated_wide_224x224.mrc |
There was a problem hiding this comment.
Remove tests from here, as it's the mask for the regular results. It agrees with the directories on OSF.
| wget https://files.osf.io/v1/resources/8h6fz/providers/dropbox/Ground_truth/mask_dilated_wide_224x224.mrc?download=true -O data/Ground_truth/mask_dilated_wide_224x224.mrc | ||
| wget https://files.osf.io/v1/resources/8h6fz/providers/dropbox/tests/Ground_truth/test_metadata_10.csv?download=true -O tests/data/Ground_truth/test_metadata_10.csv | ||
| wget https://files.osf.io/v1/resources/8h6fz/providers/dropbox/tests/Ground_truth/1.mrc?download=true -O tests/data/Ground_truth/1.mrc | ||
| wget https://files.osf.io/v1/resources/8h6fz/providers/dropbox/Ground_truth/mask_dilated_wide_224x224.mrc?download=true -O tests/data/Ground_truth/mask_dilated_wide_224x224.mrc |
There was a problem hiding this comment.
remove tests from here to be consistent.
tutorials/2_tutorial_svd.ipynb
Outdated
There was a problem hiding this comment.
Strongly suggested: Include rendered plots (low dpi) in this showing some results. People can compare against this when they reproduce things locally.
tutorials/3_tutorial_map2map.ipynb
Outdated
There was a problem hiding this comment.
what did you change in this?
There was a problem hiding this comment.
what did you change in this?
|
@DSilva27 There are some merge conflicts to attend to. But after that, since all the tests are passing, why don't you merge this in? |
|
Making an overhaul of how the normalization is done. Will close this for now. |
Included power spectrum normalization in
_preprocessing/normalize.py. I included functions to compute power spectrum and also to normalize a set of volumes. Tried to use NUFFT for a better estimation of the power spectrum, but I couldn't manage to go back to real space. I think you are not supposed to go back anyway, as the adjoint transform is not necessarily the inverse transform. If you know how to revert NUFFT, then I would prefer to implement the normalization using that.