-
Notifications
You must be signed in to change notification settings - Fork 9
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
Changed shift method in ZLP subpixel align to scipy.ndimage.shift wit… #29
Changed shift method in ZLP subpixel align to scipy.ndimage.shift wit… #29
Conversation
…h linear interpolation to get rid of artifacts.
FFT shift was chosen to minimize edge artifacts, keep statistics reasonable, and to be more precise -- so this change may solve one case only to make another case worse. For the referenced image, can you include the input data and describe how the artifacts arise? What about test cases? (see next paragraph) 1D (and 2D) shifting is provided by Core.py. Can we spend the time implementing/testing the algorithm there and then use that version instead of reimplementing in eels_analysis? (bigger picture: do we need to allow users to choose algorithm details such as what type of alignment to use?) |
I agree that we should use the shift function from Core.py. However I can't really see the benefit of fftshift: fftshift wraps edges around the image, which is something you can also tell the other shift functions to do. And how exactly is fftshift more precise than real space shift with appropriate interpolation? |
"Allow choice of Fourier shift or regular shift in sequence_align functions " nion-software/niondata#9 |
If that means that we want to give the user the option to choose the shift method then we should probably do the same for AlignZLP since it is a very similar operation |
Yes, but then the menu might not be sufficient. There will be 3 algorithm varieties (max, com, peak fit) and 2 sub varieties (fourier shift or linear shift) and possibly other variations (edge handling, for instance). So in this case, maybe linear shift with sensible edge handling is just the default for now (as submitted in this patch) until we have some UI mechanism to allow the user to more easily specify variations (yes, I know we could do it in the inspector, but that all needs an overhaul before going too much further with it). How are the edges handled in scipy.shift? |
Sounds good. Right now the edges are set to "constant" (default) which will
fill with zeros by default. But you can choose between different options
like wrap, mirror etc
Chris Meyer <[email protected]> schrieb am Do., 3. Okt. 2019, 19:34:
… Yes, but then the menu might not be sufficient. There will be 3 algorithm
varieties (max, com, peak fit) and 2 sub varieties (fourier shift or linear
shift) and possibly other variations (edge handling, for instance).
So in this case, maybe linear shift with sensible edge handling is just
the default for now (as submitted in this patch) until we have some UI
mechanism to allow the user to more easily specify variations (yes, I know
we could do it in the inspector, but that all needs an overhaul before
going too much further with it).
How are the edges handled in scipy.shift?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#29?email_source=notifications&email_token=AD7SKK3MGVUKZLJSQHLN6S3QMYUJLA5CNFSM4I4UIADKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEAI7D3I#issuecomment-538046957>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AD7SKK3HA2TR6DOCQUA5HKTQMYUJLANCNFSM4I4UIADA>
.
|
Merged. Minor point: HyperSpy uses interpolate.interp1d. |
…h linear interpolation to get rid of artifacts.
Matt showed me a dataset that had some artifacts in it after using subpixel ZLP align (see here: https://drive.google.com/file/d/1vIZJBwQRZIoZhSbFn_wwN7BzqfSZ8YIX/view?usp=sharing). This periodic pattern that is on top of the data comes from the scipy.ndimage.fftshift method. When using scipy.ndimage.shift with interpolation oder 1 (i.e. linear interpolation) the artifacts are gone. I don't think this interpolation method has any negative effects, so it should be safe to merge.