cctbx / dxtbx

Diffraction Experiment Toolbox
BSD 3-Clause "New" or "Revised" License
2 stars 18 forks source link

EIGER X 16M at SPring-8 BL41XU #675

Closed biochem-fan closed 7 months ago

biochem-fan commented 9 months ago

We got another EIGER problem. After they updated FileWriter in this March, files from EIGER X 16M are not properly read (tested with DIALS 3.17.0).

As you can see below, modules at the bottom look truncated in dials.image_viewer. image

image_size in the imported.expt is swapped.

Moreover, dials.image_viewer displays invalid pixels in module gaps differently.

I asked beamline scientists to upload a test dataset. Meanwhile, have you seen this in EIGER from other beam lines?

graeme-winter commented 9 months ago

This is the same problem as I recently had to fix for Pilatus 4; probably trivial fix (once example data available)

graeme-winter commented 9 months ago

Investigating in #676

biochem-fan commented 9 months ago

Thanks. Indeed https://github.com/cctbx/dxtbx/pull/663#issuecomment-1790353123 looks the case.

biochem-fan commented 9 months ago

The beam line scientist also says:

Did this happen on PILATUS too?

graeme-winter commented 9 months ago

The beam line scientist also says:

  • Old: fast_pixel_direction [1 0 0], slow_pixel_direction [0 1 0]
  • New: fast_pixel_direction [-1 0 0], slow_pixel_direction [0 -1 0]

Did this happen on PILATUS too?

I didn't need these changes for the Pilatus data: they correspond to a 180° rotation of the instrument around the beam, which feels ... wrong. I would wonder if something else changed which this is compensating for?

Access to the data would allow this to be assessed, so I appreciate any effort you can contribute to help make data available.

biochem-fan commented 9 months ago

@graeme-winter

Access to the data would allow this to be assessed, so I appreciate any effort you can contribute to help make data available.

The beam line scientist agreed to upload a test dataset. I will let you know once it is online.

biochem-fan commented 9 months ago

@graeme-winter The test datasets are ready.

Note that SPring-8 uses the "inverse phi" geometry. The data acquisition system at SPring-8 records this as /entry/sample/goniometer/omega.vector = [-1, 0. 0]. This value used to be compatible with earlier versions of DIALS but after this change https://github.com/cctbx/cctbx_project/issues/282, this became incorrect. The right vector in a NeXus file (McStas coordinate system) is [1, 0, 0], which DIALS reads as [-1, 0, 0]. I reported this to the beam line scientists and asked them to write [1, 0, 0] instead. Meanwhile, please import with goniometer.axis=-1,0,0.

Regarding:

Old: fast_pixel_direction [1 0 0], slow_pixel_direction [0 1 0] New: fast_pixel_direction [-1 0 0], slow_pixel_direction [0 -1 0]

I could not find any old datasets with positive vectors. I will double check with the beam line scientist.

graeme-winter commented 9 months ago

@graeme-winter The test datasets are ready.

  • XRD-00203 : Lysozyme collected on EIGER X2 CdTe 4M at SPring-8 BL41XU (EIGER FileWrite version release-2022.1.2)
  • XRD-00204: Thaumatin collected on EIGER X 16M at SPring-8 BL41XU (EIGER FileWriter version 2022.1.2)

Note that SPring-8 uses the "inverse phi" geometry. The data acquisition system at SPring-8 records this as /entry/sample/goniometer/omega.vector = [-1, 0. 0]. This value used to be compatible with earlier versions of DIALS but after this change cctbx/cctbx_project#282, this became incorrect. The right vector in a NeXus file (McStas coordinate system) is [1, 0, 0], which DIALS reads as [-1, 0, 0]. I reported this to the beam line scientists and asked them to write [1, 0, 0] instead. Meanwhile, please import with goniometer.axis=-1,0,0.

Regarding:

Old: fast_pixel_direction [1 0 0], slow_pixel_direction [0 1 0] New: fast_pixel_direction [-1 0 0], slow_pixel_direction [0 -1 0]

I could not find any old datasets with positive vectors. I will double check with the beam line scientist.

Noted, thank you, downloading

biochem-fan commented 8 months ago

@graeme-winter

With the following patch, I confirmed that datasets above can be processed fine.

diff --git a/src/dxtbx/format/FormatNXmxEigerFilewriter.py b/src/dxtbx/format/FormatNXmxEigerFilewriter.py
index fbfde292..96d4fa1e 100644
--- a/src/dxtbx/format/FormatNXmxEigerFilewriter.py
+++ b/src/dxtbx/format/FormatNXmxEigerFilewriter.py
@@ -49,14 +49,12 @@ class FormatNXmxEigerFilewriter(FormatNXmx):
         if nxdetector.underload_value is None:
             nxdetector.underload_value = 0

-        # data_size is reversed - we should probably be more specific in when
-        # we do this, i.e. check data_size is in a list of known reversed
-        # values
-        known_safe = [
-            (1082, 1035),
-        ]
+        # data_size was reversed in files earlier than release-2022.1.2.
+        # https://github.com/cctbx/dxtbx/pull/663#issuecomment-1790353123
+        fw_version = nxmx.h5str(self._cached_file_handle["/entry/instrument/detector/detectorSpecific/eiger_fw_version"][()])
+        is_inverted = fw_version < "release-2022.1.2"
         for module in nxdetector.modules:
-            if not tuple(module.data_size) in known_safe:
+            if is_inverted:
                 module.data_size = module.data_size[::-1]
         return nxmx_obj

is_inverted = fw_version < "release-2022.1.2" is not safe for firmware versions like beta-2020.1.1 (are there such cases?).