Hi Yujin,

a few ideas:

• I don't think you need to do the correction in 3D. The focus lock should be keeping it stable in z, so you could probably just do the correction in x and y. This should improve memory usage and performance.
• Sorry I didn't mention it earlier, but there is also ndimage.shift() which might be easier / less overhead than .map_coordinates()
• It would be really good to avoid duplication on the event parsing. I think we should either use pipeline.processEvents() or (at a pinch) just copy lines 259 ad 261 from pipeline.processEvents(). There is no reasontime_to_frames() shouldn't work in single-shot acquisition mode (although we are reliant on having the events from the OIDIC process to get frame timings, so we can't just copy the events from the SMLM process and call it done - this should shake out soon with the spooling refactor).
• We can safely ignore the image origin when doing drift correction (no need to add it to the coordinates)
• Ideally we'd implement as a recipe module rather than PYMEImage plugin.
• I'm not sure on this, but it might make sense to break into 2 modules - one to get the shift, and one to do the correction (for easier re-use if, e.g. someone wanted to correct the shift from fiducials). We can always get something working then come back to this.

If we are not doing 3D correction, it might be elegant to implement as a DataSource similar to BGSDataSource.XYZTCBgsSource e.g.

class XYZTCDriftCorrectSource(XYZTCDataSource):
    moduleName = 'DriftCorrectDataSource'
    
    def __init__(self, datasource, x_mapping, y_mapping, x_scale=1.0, y_scale=1.0):
        if (not isinstance(datasource, XYZTCDataSource)) and (not datasource.ndim == 5) :
            datasource = XYZTCWrapper.auto_promote(datasource)
        
        self._datasource = datasource
        size_z, size_t, size_c = datasource.shape[2:]

        self._x_map = x_mapping # a piecewise mapping object
        self._x_scale = x_scale #allows conversion between different camera pixel units. TODO - make it an affine transformation matrix to allow for rotation as well.
        self._y_map = y_mapping
        self._y_scale = y_scale
        
        XYZTCDataSource.__init__(self, input_order=datasource._input_order, size_z=size_z, size_t=size_t, size_c=size_c)
    
    def getSlice(self, ind):
        sl = self._datasource.getSlice(ind)
        return ndimage.shift(sl, [self._x_scale*self._x_map(ind), self._y_scale*self._y_map(ind)]) 
    
   # proxy original data source attributes 
    def __getattr__(self, item):
        return getattr(self._datasource, item)
        
    def getSliceShape(self):
        return self._datasource.getSliceShape()

    def getNumSlices(self):
        return self._datasource.getNumSlices()

    def getEvents(self):
        return self._datasource.getEvents()

    @property
    def is_complete(self):
        return self._datasource.is_complete()

The advantages of this method is that it should work regardless of the dimensionality and dimension order of the input (i.e. it should work for z stacks and time sequences etc ... without any futzing around). The potential caveat is that it is lazily computed (i.e. computed on access). This means that it will return pretty much instantly, but there will be a lag when each frame is accessed - depending on how fast ndimage.shift is this could either be a good or a bad thing. If lazy computation is not desired, the computation can easily be forced by doing, e.g.

ds = XYZTCDriftCorrectSource(...)
ImageStack(ds.data_xyztc[:,:,:,:,:], mdh=ds.mdh)

rather than

ImageStack(XYZTCDriftCorrectSource(...))

You guys will probably call it nitpicking but I would vote against any print() calls in production code. While ok for development we should stick to the logging facility for anything to be kept in PYME proper and issue these at the desired log level.

@csoeller - definitely a good idea to limit / phase out print()s, but think we're a bit early to be worrying about that on this one. I'm probably the chief offender for letting them slip into production, so might have to think about an automatic test for them ... maybe something along the lines of:

git diff upstream/master -G '^\s*print\(' -U0 --exit-code

although that one will pick up both additions and deletions ...

@csoeller - maybe git diff upstream/master -G '^\s*print\(' -U0 | grep -e '^+\s*printf(' to just pickup additions ...

No worries re print etc. More generally in relation to this PR, why would one want to drift correct the source images. Presumably this is not for images eventually intended for SMLM?

The application here is a bit complicated, but essentially boils down to taking simultaneous (or interleaved) SMLM and OIDIC images, and correcting the non SMLM channel. In principle it could also be used for widefield fluorescence / SIM / etc ...

Thanks @David-Baddeley @csoeller for the suggestions! It is definitely necessary to limit print()s. Sorry for my knowledge limitation at coding, but this is the easiest way to help me with debugging. Now I remove the print()s and the code has been modified as suggested. A DataSource has been created and it has been implemented as a recipe module. Please let me know if you have any further thoughts.

Hi David, since my fluorescence channel has scaling and rotation relative to the OI-DIC (also the drift tracking) channel, I modified the code to generalize the correction using an affine transformation as suggested in XYZTCDriftCorrectSource by you. I separate the x and y pixel sizes in case they are different, while I am not sure if this is worthwhile since such case may be rare. Please let me know if the variable settings are reasonable. Thanks!

Hi @Yujin-Bao - sorry it's taken so long to have a good look at this. Main potential issues (as mentioned in comments on the code) are as follows:

• Need to support flipping between cameras, and a rotation origin (i.e. full affine transform)
• Need to look at interface consistency with drift correction of point data (this may not be breaking)