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

Changed shift method in ZLP subpixel align to scipy.ndimage.shift wit… #29

Conversation

Brow71189
Copy link
Collaborator

…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.

…h linear interpolation to get rid of artifacts.
@cmeyer
Copy link
Contributor

cmeyer commented Oct 3, 2019

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?)

@Brow71189
Copy link
Collaborator Author

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?
You get the artifacts when you align the raw data with Align ZLP (com) or Align ZLP (peak fit) and the nintegrate the sequence. Here is the raw data:
https://drive.google.com/file/d/1DGGXTso6NNTbyEib9PNNdBtPreXUcqrr/view?usp=sharing
Changing the shift method gets rid of them, so does Align ZLP (max) which is directly doing integer pixel shifts.

@cmeyer
Copy link
Contributor

cmeyer commented Oct 3, 2019

"Allow choice of Fourier shift or regular shift in sequence_align functions " nion-software/niondata#9

@Brow71189
Copy link
Collaborator Author

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

@cmeyer
Copy link
Contributor

cmeyer commented Oct 3, 2019

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?

@Brow71189
Copy link
Collaborator Author

Brow71189 commented Oct 3, 2019 via email

@cmeyer cmeyer merged commit 710a798 into nion-software:master Oct 3, 2019
@cmeyer
Copy link
Contributor

cmeyer commented Oct 3, 2019

Merged. Minor point: HyperSpy uses interpolate.interp1d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants