Add Fourier and Interp shift functions #213
Conversation
To allow testing different implementations of the subpixel shift. Curently the version to use must be hard coded in the __getitem__ and __setitem__ methods of the Storage class. Also there are no proper checks of dtype and current testing is only using 2D Storages.
ptypy/core/classes.py
Outdated
| @@ -1046,9 +1050,14 @@ def __getitem__(self, v): | |||
| # v.roi[0, 1]:v.roi[1, 1]], v.sp) | |||
| if isinstance(v, View): | |||
| if self.ndim == 2: | |||
| return shift(self.data[ | |||
| if np.any(v.sp != 0.0): | |||
| return shift_fourier(self.data[ | |||
There was a problem hiding this comment.
We should be able to set a switch on the storage that switches it to use subpixel interpolation. We only need it for the object storage get and set.
…l interp for this storage
|
Need to check the build, and fix a couple of issues, but @bjoernenders @pierrethibault are you generally happy with this approach. It sort of feels like this is how you designed it to be... let me know if I need to change anything. |
ptypy/core/classes.py
Outdated
| @@ -1096,7 +1096,7 @@ def __setitem__(self, v, newdata): | |||
| if np.any(v.sp != 0.0): | |||
There was a problem hiding this comment.
I think, it is better to put a low_level threshold here, like a certain percentage of a pixel. Realistically, v.sp will never be zero.
ptypy/core/classes.py
Outdated
| @@ -34,7 +34,11 @@ | |||
| """ | |||
| import numpy as np | |||
| import weakref | |||
| import scipy.ndimage.interpolation | |||
There was a problem hiding this comment.
I do not like scipy imports in core.classes
ptypy/core/classes.py
Outdated
| elif self._shift_type is 'interp': | ||
| self._subpixel_shift = shift_interp | ||
| else: | ||
| raise RuntimeError('%s is is supported for subpixel_shifting' % val) |
There was a problem hiding this comment.
This should give a warning and deactivate subpixel shift. Or at least tell which options are valid
ptypy/core/classes.py
Outdated
| @@ -1127,6 +1157,37 @@ def shift(v, sp): | |||
| return v | |||
|
|
|||
|
|
|||
| def shift_fourier(v, sp, antialiasing_factor=1, report=False): | |||
|
Hi @bjoernenders. This is an edge case I agree, but it is one I see more and more often on nanoprobe beamlines where the improvement to the resolution by doing ptycho is small (but important). Also where you want to collect high resolution XRF at the same time so need to work at focus. In this case your step size is close to pixel size and the granularity makes the algorithm blow up. I guess an alternative to subpixel shifts would be to zero pad the data but this would be more computationally costly. I'll work on an example to show you what I mean. |
|
HI @bjoernenders , I briefly talked about this with @pierrethibault the other day, and was thinking about finishing this off. We now have plenty of data that doens't reconstruct/ has to be hack in order to get around this not existing. The sticking point is that we want to shift the band limiteded probe not the object since it's more inline with the physics/ less prone to aliasing. An initial implementation of this could be done by copying the .sp from the object to the probe in the model initialisation. The "problem" comes when trying to integrate this with position correction since you then have two set's of co-ordinates that you need to keep in sync at every point. It seems to me that making this synchronization something that the model does makes sense. And, thinking about it would mean that instead of having a PositionCorrectionEngine to inherit from, we should have instead extended the model. The model then gains a method to update it's own database, rather than reaching in from the engine. @pierrethibault @bjoernenders does this make sense or am I just rambling? I'll have a go at getting something together over the next couple of days unless you say otherwise. |
|
Whilst we are at it, since in the _cread_pods method basically has the rows of the table being created (which I am suggesting the model should own), we should just make this directly fill in the address book we are using for the gpu work. We can apply masks to this database if subsets are needed instead. What do you think? |
|
@aaron-parsons and @bjoernenders : I agree that this is ScanModel's responsibility, and that it would be appropriate to have a mechanism to apply the shift to the probe instead of the object. |
…ack it, but can have preloaded functions.
| p.scans.MF.data.density = 0. | ||
| # total number of photon in empty beam | ||
| p.scans.MF.data.photons = 1e8 | ||
| p.scans.MF.data.use_subpix = True |
There was a problem hiding this comment.
Shouldn't this be p.scans.MF.use_subpix?
There was a problem hiding this comment.
No. This is inside the moonflower scan subclass and means that the pixels aren’t rounded in the simulation.
DO NOT MERGE YET
This is Joes work that needs finishing off. I put it up here for comments.
To allow testing different implementations of the
subpixel shift.
Curently the version to use must be hard coded in the
getitem and setitem methods of the Storage class.
Also there are no proper checks of dtype and current testing
is only using 2D Storages.