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

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

Open
tjlane opened this issue May 14, 2020 · 12 comments
Open

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

tjlane opened this issue May 14, 2020 · 12 comments
Assignees

Comments

@tjlane
Copy link
Contributor

tjlane commented May 14, 2020

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
Copy link
Contributor Author

tjlane commented May 14, 2020

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 added a commit that referenced this issue May 14, 2020
@tjlane
Copy link
Contributor Author

tjlane commented May 14, 2020

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
Copy link
Collaborator

valmar commented May 15, 2020

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
Copy link
Collaborator

valmar commented May 15, 2020

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

@tjlane
Copy link
Contributor Author

tjlane commented May 15, 2020

@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
Copy link
Collaborator

valmar commented May 15, 2020 via email

@tjlane
Copy link
Contributor Author

tjlane commented May 15, 2020

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
Copy link
Collaborator

valmar commented May 15, 2020 via email

@tjlane tjlane self-assigned this Jun 16, 2020
tjlane added a commit that referenced this issue Jun 19, 2020
tjlane added a commit that referenced this issue Jun 19, 2020
@tjlane
Copy link
Contributor Author

tjlane commented Jun 19, 2020

@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
Copy link
Contributor Author

tjlane commented Jun 19, 2020

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
Copy link
Collaborator

valmar commented Jun 26, 2020

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

@tjlane
Copy link
Contributor Author

tjlane commented Jun 26, 2020

@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!

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

No branches or pull requests

2 participants