-
Notifications
You must be signed in to change notification settings - Fork 4
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
Position to data coords #210
base: master
Are you sure you want to change the base?
Conversation
- default origin to (snapped) detector centre - input data formated as (x, y) positions (is this the best?)
xx, yy = np.meshgrid( | ||
np.arange(self.expected_data_shape[-1]), | ||
np.arange(np.prod(self.expected_data_shape[:-1])), | ||
indexing='xy' | ||
) | ||
pxx = self.position_modules(xx.reshape(self.expected_data_shape).astype(float))[0] | ||
pyy = self.position_modules(yy.reshape(self.expected_data_shape).astype(float))[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm following correctly, these 'x' and 'y' things should really be 'fast scan' and 'slow scan'. E.g. for AGIPD, the fast scan direction, which you've called 'x' here, is typically along the y axis.
(I know I sound like a broken record about this, but geometry code is hard to follow at the best of times, so I really want to avoid making it harder with half-right names)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the x and y coordinates in a (2D) data array. I can call them fast and slow scan if you prefer but I found it more confusing as it is not the same as what we call ss/fs usually in this code base. And in this case x will always be fast scan and y slow scan.
If unit is None (default) the positions given are in pixel, else given positions | ||
are in meter relative to the centre, e.g. `unit=1e-3` interprets positions in mm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For new methods like this, where we can be consistent across all detectors, how would you feel about saying the input is always in metres, and if you've got e.g. data in pixel units, you apply the unit like data * geom.pixel_size
?
subpixel offset of tile or rotation in your geometry will be ignored. | ||
|
||
Returns 4 array of similar length for module, slow scan and fast scan | ||
coordinates and a sub-array of the input data excluding invalid coordinates. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sub-array excluding invalid coordinates feels a little bit janky. What about a boolean array which is True for valid coordinates and False for invalid ones? I think that makes it easy to match up the input with the output if needed.
.. note:: | ||
|
||
This uses an assembly limited to snapping pixel to a regular grid, so any | ||
subpixel offset of tile or rotation in your geometry will be ignored. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be possible - and not much more complex - to do this 'properly', preserving subpixel offsets and rotations and so on. Something like (pseudocode):
for mod in modules:
for t, tile in enumerate(mod):
ss_coord, fs_coord = find_data_coords_for_tile(data, tile)
on_tile = (0 < ss_coord) & (ss_coord < tile.ss_pixels) & (0 < fs_coord) & (fs_coord < tile.fs_pixels)
ss_coord, fs_coord = tile_coords_to_module(ss_coord[on_tile], fs_coord[on_tile], t)
# Record these results to result arrays
I'm assuming the numbers of tiles per detector are small enough to get away with just checking each tile one by one. So far it's a max of 128 tiles (LPD-1M), IIRC. If that's too slow, we'd need a bit more complexity to find the relevant tiles efficiently, but hopefully not much more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hum, the issue I can see is if one relies on our (snapped) assembled data to find coordinates, using the subpixels/rotated positions here might lead to wrong coordinate mapping :/
FYI, I did something vaguely similar for MID recently. They were drawing small ROIs on an assembled image, so to avoid loading/assembling all the modules I wrote a function that took in the ROI bounds and created a mask in array coordinates: # Helper function to create a mask in an unassembled (module, y, x)
# shape from the ROI bounds in an assembled image.
def generate_mask(geom, xmin, xmax, ymin, ymax):
# Create a LUT, a flat array the same size of a data array where the elements are the assembled-array indices of the same pixel.
lut_data = np.empty((16, 512, 128))
lut_data.flat[:] = np.arange(lut_data.size)
assembled, _ = geom.position_modules(lut_data)
lut = np.argsort(assembled.flat)[:lut_data.size].astype(np.uint64)
# Create an assembled mask image from ROI coordinates
assembled_mask = np.full_like(assembled, False, dtype=bool)
assembled_mask[ymin:ymax, xmin:xmax] = True
assembled_mask[np.isnan(assembled)] = False
# Create a mask in array coordinates and use the LUT to assign the masked elements
mask = np.full((16, 512, 128), False, dtype=bool)
mask.flat[assembled_mask.flat[lut]] = True
# Find the modules used in the mask
module_numbers = np.any(mask, axis=(1, 2)).nonzero()[0]
return mask[module_numbers], module_numbers This was specific to the AGIPD and a rectangular ROI, but the same principle should work for any geometry and mask. I wonder if it would be simpler to add this thing as a generic |
Find pixels coordinates in data array from the positions on an assembled image.
e.g. visualization of pixel coordinates from ring positions given by pyfai:
regular grid around the centre: