Skip to content

Fle basis odd patch #858

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

Merged
merged 4 commits into from
Feb 17, 2023
Merged

Fle basis odd patch #858

merged 4 commits into from
Feb 17, 2023

Conversation

garrettwrong
Copy link
Collaborator

I think this closes #856 , let me know if you agree.

@garrettwrong garrettwrong added bug Something isn't working enhancement New feature or request cleanup labels Feb 17, 2023
@garrettwrong garrettwrong requested a review from janden as a code owner February 17, 2023 03:38
@garrettwrong garrettwrong self-assigned this Feb 17, 2023
@codecov
Copy link

codecov bot commented Feb 17, 2023

Codecov Report

Merging #858 (1c6fe2b) into fle_basis (4e67056) will decrease coverage by 0.31%.
The diff coverage is 100.00%.

@@              Coverage Diff              @@
##           fle_basis     #858      +/-   ##
=============================================
- Coverage      89.20%   88.90%   -0.31%     
=============================================
  Files            115      115              
  Lines           9814     9368     -446     
=============================================
- Hits            8755     8329     -426     
+ Misses          1059     1039      -20     
Impacted Files Coverage Δ
src/aspire/basis/fle_2d.py 100.00% <100.00%> (ø)
src/aspire/source/coordinates.py 92.43% <0.00%> (-1.22%) ⬇️
src/aspire/utils/__init__.py 100.00% <0.00%> (ø)
src/aspire/source/relion.py 96.90% <0.00%> (+0.07%) ⬆️
src/aspire/source/image.py 97.47% <0.00%> (+0.24%) ⬆️
src/aspire/utils/misc.py 92.79% <0.00%> (+0.90%) ⬆️
src/aspire/numeric/complex_pca/complex_pca.py 92.30% <0.00%> (+3.84%) ⬆️
src/aspire/utils/relion_interop.py 100.00% <0.00%> (+6.34%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@chris-langfield
Copy link
Collaborator

Yes, checked it out a bit in the notebook as well. This seems to be the fix. Also, the fixed h parameter fixes a normalization issue between FB and FLE slow for odds. Just added a commit to remove those too. Thanks! I'm really glad this ended up being resolved and I'm not surprised it was that simple 😁

@garrettwrong
Copy link
Collaborator Author

Oh cool, so this closes both issues?! #738 too?

@garrettwrong garrettwrong merged commit 3c22ce8 into fle_basis Feb 17, 2023
@chris-langfield
Copy link
Collaborator

No, the result from the dense matrix still gives some of the basis functions / coefficients flipped. The odd resolution ones also had a different normalization (after taking absolute value) from FB which this patch fixes. Thanks again!

@garrettwrong garrettwrong deleted the fle_basis_odd_patch branch June 1, 2023 18:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cleanup enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants