Compatibility with cfelpyutils 2.x & read/write bad regions from .geom file

[takluyver]

Switch from cfelpyutils 1.x to 2.x. I'm not attempting to bridge the gap and make it work with both - though we could, if there's a need for that.

One minor concrete benefit is that we now get photon energy from the .geom file, so we can store that and write it back along with some other values.

[takluyver]

These fields appear to have been renamed in cfelpyutils. I couldn't work out why by looking at the code, and I didn't get any answer from CFEL when I asked. 🤷

[takluyver]

I've now extended this to try to read the bad (mask) regions from a .geom file, store them in the metadata, and write them back out when saving back to a .geom file. This is considerably more complex, because the bad regions can refer to the panel names, which we don't store, and their definition depends on the data layout (2D vs 3D), which we abstract away when reading the geometry.

The mask from the .geom file is not automatically applied when assembling data. My plan is to integrate this with the work in #112, which can convert a collection of rectangular regions into an array mask.

[turkot]

I think in extra_geom/crystfel_fmt.py we should modify lines 8-14:

HEADER_TEMPLATE = """\
; {detector} geometry file written by EXtra-geom {version}
; You may need to edit this file to add:
; - data and mask locations in the file
; - mask_good & mask_bad values to interpret the mask
; - adu_per_eV & photon_energy
; - clen (detector distance)

since now only mask_good & mask_bad are not copied from the original geometry file and everything else is.

[turkot]

I have tried to read the JUNGFRAU geometry and then to write it back into the new geometry file.
From one hand it seem to have worked - CrystFEL accepted the file and 9609 frames were indexed.
From the other hand with original file and exactly the same parameters number of indexed frames was 9627, so there is some small difference.

While looking into the files I can see that convention for the panels and asics naming got broken, e.g.:
p1a1 -> p0a7;
p1a2 -> p0a6;
p1a3 -> p0a5;
p1a4 -> p0a4;
p1a5 -> p0a3;
p1a6 -> p0a2;
p1a7 -> p0a1;
p1a8 -> p0a0.
But it was done consistently for panels positions, rigid groups and bad regions, so it is fine.

LGTM 👍

[takluyver]

So was the geometry file you tested with starting from p1a1? And what are the ss/fs coordinates for p1a1?

The AGIPD .geom file I have starts - agipd_mar18_v11.geom with p0a0, and each p*a0 starts from min_fs = 0 min_ss = 0 (so it's a0-a7 along the slow-scan dimension of each module). This is what I've tried to emulate in the writing code, though I haven't actually checked reading that file and writing it out recently.

[turkot]

I think this image could explain the very weird naming scheme JUNGFRAU panels have. So panels and asics indices start with 1 instead of 0, for panels 1-4 (min_fs = 0, min_ss = 0) asic is 8, but for panels 5-8 - it is 1.

But as I have mentioned - renaming have been done consistently everywhere in the new geometry file, so I don't think it is a big problem.

[takluyver]

Got it, thanks. That looks like a really good test case that this machinery is working correctly. The ASICs for modules 1-4 will be renumbered in reverse, but for 5-8 just shifted to count from 0 rather than 1.

Separate from this PR, we could have an offset so JUNGFRAU modules are numbered from 1 rather than from 0, if you think that would make the resulting .geom files easier to understand. But preserving that numbering scheme of tiles within modules would be trickier, and I don't want to spend too much time on roundtripping .geom files.

I think in extra_geom/crystfel_fmt.py we should modify... since now only mask_good & mask_bad are not copied from the original geometry file and everything else is.

This is a valid point, but it's only true if you start from a .geom file. You can make a geometry object with e.g. .from_quad_positions() and then write it out to a .geom file, or read a .h5 geometry file and write out a .geom file. In those cases, it will still be missing all those things. So the comment pedantically says that you may need to set them.

I wonder if we should have a separate method which can modify an existing .geom file rather than writing out a completely new one. That would also preserve the panel naming and any comments in the file. 🤔

[turkot]

Right, thank you, indeed there is no need to modify the comment.
I don't think you should modify indexing for JUNGFRAU to start from 1.
What potentially could be done is to store original panels names from the geometry file which is read as a dictionary in metadata, mapping internal panel names to the original, and during writing - write the original names from the dictionary.
But of course it will work only when the geometry is read from the file, and in my opinion it is not a priority, since everything already works well.

Maybe a better option would be to change the naming convention to have it the same for all detectors?) Right now it is pretty hard to understand JUNGFRAU layout without an image.

[takluyver]

I wish we could make the naming/numbering convention the same for all detectors! But it's not something we have much control over.