slaclab / psgeom

code for representing the geometry of scattering experiments
Other
2 stars 3 forks source link

BUG: Overlap in crystfel geometry for JF, possibly others #41

Open tjlane opened 4 years ago

tjlane commented 4 years ago

From Valerio: max_fs and max_ss are inclusive. This means that, for example pixel ss 255 belongs to p0, but as you can see, it is also the first ss pixel of p1, so that pixel appears in two panels.

p0/fs = +0.000000x +1.000000y
p0/ss = -1.000000x +0.000000y
p0/res = 13333.333
p0/corner_x = 257.000000
p0/corner_y = -515.000000
p0/coffset = 0.010000
p0/min_fs = 0
p0/max_fs = 255
p0/min_ss = 0
p0/max_ss = 255
p0/no_index = 0

p1/fs = +0.000000x +1.000000y
p1/ss = -1.000000x +0.000000y
p1/res = 13333.333
p1/corner_x = 257.000000
p1/corner_y = -257.000000
p1/coffset = 0.010000
p1/min_fs = 0
p1/max_fs = 255
p1/min_ss = 255
p1/max_ss = 510
p1/no_index = 0
tjlane commented 4 years ago

Actually, the problem goes deeper: if we use this file provided by Anton as an example of the data layout expected:

https://github.com/slaclab/psgeom/blob/master/ref_files/jungfrau/jungfrau-1M-pan.geom

we need to format the min/max fs/ss flags to match the (4x8) ASIC layout

tjlane commented 4 years ago

So, to do:

  1. Name the panels pXaY for panel X asic Y (presumably)
  2. Add rigid group flags, such as rigid_group_p1 = p1a1,p1a2,p1a3,p1a4,p1a5,p1a6,p1a7,p1a8
  3. Fix min/max_fs/ss assuming that data will be stacked along the ss direction

To do this we need to figure out the layout of the panel assembly for each basis_grid grid element here:

https://github.com/slaclab/psgeom/blob/master/psgeom/translate.py#L816

For each grid element, we need to look back at the detector object and find out

  1. What sensor (child) it corresponds to [as an index]
  2. For that sensor, which sub-panel it corresponds to

Correspondingly, we need two methods,

  1. a new method on CompoundCamera that maps: bg_index --> leaf_index, panel_in_leaf_index. Then we can look up the leaf (a sensor object). [done : CompoundAreaCamera._bg_index_to_camera_index]
  2. These sensors need one more property that gives their sub-panel shape, e.g. (2,4) for JF. Using this we can lay out the data correctly. [done : subpanel_shape]
valmar commented 4 years ago

Be aware that currently CrystFEL (More precisely Cheetah) cannot deal with a 3-d array (for example 8x512x1024), but only with a 2-d array (called the "slab"). What is usually done is that the array is "flattened" along the slowest scan coordinate. For example: 8x512x1024 -> 4096x1024

valmar commented 4 years ago

However, I would generate an array that matches the real geometry, and maybe leave the flattening to a separate script or the user

tjlane commented 4 years ago

@valmar right... I don't think we should mess with formatting the raw data here. But we can strive to write a crystfel geometry that anticipates "the slab" format. Do you think that makes sense?

valmar commented 4 years ago

What I mean is that if the data, like in this case, comes as a 8x512x1024 array, the geometry should describe that, then it is enough to flatten the array in the geometry file to correctly read the raw data (because we flatten along the slowest axis!)

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Friday, May 15, 2020 5:27 PM, TJ Lane notifications@github.com wrote:

@valmar right... I don't think we should mess with formatting the raw data here. But we can strive to write a crystfel geometry that anticipates "the slab" format. Do you think that makes sense?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

tjlane commented 4 years ago

yes OK we agree! sounds good. The plan above basically sketches how to do this. Do you need it urgently? Otherwise I will probably get to it early/mid next week.

valmar commented 4 years ago

No, no for my tests I can create a geometry file manually, it's not (yet) for use on real data.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐ On Friday, May 15, 2020 5:49 PM, TJ Lane notifications@github.com wrote:

yes OK we agree! sounds good. The plan above basically sketches how to do this. Do you need it urgently? Otherwise I will probably get to it early/mid next week.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

tjlane commented 4 years ago

@valmar when you have a moment: could you look at the current code in master and see if it produces crystfel geometries with the correct min_fs, min_ss, max_fs, max_ss fields? The CSPAD currently cheats in this regard and uses hard-coded values, so test it with a JUNGFRAU (for example).

tjlane commented 4 years ago

Last thing to do. Think about removing the bespoke code for the CSPAD:

https://github.com/slaclab/psgeom/blob/master/psgeom/translate.py#L901

in favor of the new more general code. This would be great. To do so, we would want to:

valmar commented 4 years ago

@tjlane Why did you reopen this? What is missing?

tjlane commented 4 years ago

@valmar sorry for the confusion. The main issue is resolved, there is just a little cleanup we can do that I don't want to forget about. Specifically, there are currently two functions to write crystfel geometries:

https://github.com/slaclab/psgeom/blob/master/psgeom/translate.py#L777 https://github.com/slaclab/psgeom/blob/master/psgeom/translate.py#L904

The second is a special one just for CSPADs that we now remove.

This isn't urgent but I just don't want to forget about it!