Skip to content
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

Make ifft computer really a computer #529

Merged
merged 1 commit into from Dec 5, 2017

Conversation

Projects
None yet
2 participants
@bnorthan
Copy link
Contributor

commented Dec 1, 2017

IFFT computer tries to save memory by writing temporary IFFT calcuations
to the input image. THis means the computer changes the input. We can
avoid this by making a copy of the input and using that for the temporary
calculations.

This will make the op use more memory and perform slower. In the future
an op type which allows using the input img for temporary calculations
should be created.

@bnorthan

This comment has been minimized.

Copy link
Contributor Author

commented Dec 1, 2017

@ctrueden @kkangle I submitted a potential fix to your problem. Feel free to modify it, if needed.

Note, I had to create the CreateOp in compute, this is because there is no guarantee that the pixel type of the input/output is known when initialize is called. If you know of a way to get around this let me know.

Make ifft computer really a computer
IFFT computer tries to save memory by writing temporary IFFT
calculations to the input image. This means the computer changes the
input. We can avoid this by making a copy of the input and using that
for the temporary calculations.

This will make the op use more memory and perform slower. In the future
an op type which allows using the input img for temporary calculations
should be created.

Signed-off-by: Curtis Rueden <ctrueden@wisc.edu>

@ctrueden ctrueden force-pushed the fix_ifftcomputer branch from 433f621 to ff61566 Dec 5, 2017

@ctrueden

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

Thanks a lot, @bnorthan. I force pushed a simplified version of your fix. Tests are passing, so I'll merge it now.

@ctrueden ctrueden merged commit 6170e0f into master Dec 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ctrueden ctrueden deleted the fix_ifftcomputer branch Dec 5, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.