Skip to content

Conversation

@garrettwrong
Copy link
Collaborator

This should work around the changes put in FINUFFT v2.0.4. Also it seems like their project lead might prefer to enforce this instead of casting in the future.

@garrettwrong garrettwrong added bug Something isn't working good first issue Good for newcomers CI Continuous Integration labels Feb 16, 2022
@garrettwrong garrettwrong self-assigned this Feb 16, 2022
@garrettwrong
Copy link
Collaborator Author

Closes #572

Deprecates #573

@garrettwrong
Copy link
Collaborator Author

See flatironinstitute/finufft#203

@codecov
Copy link

codecov bot commented Feb 16, 2022

Codecov Report

Merging #575 (3dcfaeb) into develop (ef91777) will decrease coverage by 0.22%.
The diff coverage is 84.61%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #575      +/-   ##
===========================================
- Coverage    87.42%   87.20%   -0.23%     
===========================================
  Files          108      108              
  Lines         8172     8000     -172     
===========================================
- Hits          7144     6976     -168     
+ Misses        1028     1024       -4     
Impacted Files Coverage Δ
src/aspire/commands/preprocess.py 0.00% <0.00%> (ø)
src/aspire/__init__.py 100.00% <100.00%> (ø)
src/aspire/basis/ffb_3d.py 100.00% <100.00%> (ø)
src/aspire/commands/extract_particles.py 85.71% <100.00%> (+5.71%) ⬆️
src/aspire/nufft/finufft.py 100.00% <100.00%> (ø)
src/aspire/source/image.py 95.43% <100.00%> (-0.28%) ⬇️
src/aspire/source/simulation.py 99.38% <100.00%> (-0.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5d7747...3dcfaeb. Read the comment docs.

@garrettwrong garrettwrong marked this pull request as ready for review February 16, 2022 23:45
@garrettwrong garrettwrong requested a review from janden as a code owner February 16, 2022 23:45
Copy link
Collaborator

@janden janden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So do I understand correctly in that 2.0.3 allowed real dtypes and did the cast to complex, but that 2.0.4 did not cast (but allowed real dtypes, hence the bug)?

@garrettwrong garrettwrong merged commit 5e2d49a into develop Feb 17, 2022
@garrettwrong
Copy link
Collaborator Author

Yes, that is correct. Prior versions of FINUFFT cast real signals to complex for the user.

@garrettwrong garrettwrong deleted the nufft_hotfix branch February 17, 2022 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working CI Continuous Integration good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants