Eiger filewriter update breaks nxmx #782
Comments
|
Hi. I am not a core developer but often work on writing (or amending) NXmx files.
Which version is it?
Indeed there is something confusing here. In 2023, I asked this to @phyy-nx when I was writing an NXmx file for our XFEL detector. I asked:
and @phyy-nx answered:
Indeed, the NXmx specification says So, I thought the starting point should be module's
The package should harmonize with the NXmx specification. It is
It is surprising to me that Dectris released an update without testing it against DIALS, a common downstream application. If Dectris believed their NXmx interpretation, not that of DIALS, was correct, which is also possible, they should have contacted developers to give them a chance to update the program before troubling end-users. (@phyy-nx & @graeme-winter, perhaps this is a good discussion point at HDRMX?) |
|
I've picked this up with colleagues at DECTRIS for discussion |
|
Verified that this is an issue with the current dials version Grey-Area lys_1 :) $ dials.import lys_1_master.h5
DIALS (2018) Acta Cryst. D74, 85-97. https://doi.org/10.1107/S2059798317017235
DIALS 3.dev.1248-g1319ea150
Traceback (most recent call last):
File "/Users/graeme/git/dials/conda_base/bin/dials.import", line 8, in <module>
sys.exit(run())
^^^^^
File "/Users/graeme/git/dials/conda_base/lib/python3.11/contextlib.py", line 81, in inner
return func(*args, **kwds)
^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dials/src/dials/command_line/dials_import.py", line 1012, in run
do_import(args, phil=phil, configure_logging=True)
File "/Users/graeme/git/dials/modules/dials/src/dials/command_line/dials_import.py", line 857, in do_import
params, options, unhandled = parser.parse_args(
^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dials/src/dials/util/options.py", line 876, in parse_args
params, args = self._phil_parser.parse_args(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dials/src/dials/util/options.py", line 558, in parse_args
importer = Importer(
^^^^^^^^^
File "/Users/graeme/git/dials/modules/dials/src/dials/util/options.py", line 207, in __init__
self.unhandled = self.try_read_experiments_from_images(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dials/src/dials/util/options.py", line 284, in try_read_experiments_from_images
experiments = ExperimentListFactory.from_filenames(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/model/experiment_list.py", line 703, in from_filenames
imageset = format_class.get_imageset(
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/FormatMultiImage.py", line 161, in get_imageset
format_instance = cls.get_instance(filenames[0], **format_kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/Format.py", line 274, in get_instance
Class._current_instance_ = Class(filename, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/FormatNXmxEigerFilewriter.py", line 29, in __init__
super().__init__(image_file, **kwargs)
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/FormatNXmx.py", line 59, in __init__
super().__init__(image_file, **kwargs)
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/FormatHDF5.py", line 16, in __init__
Format.__init__(self, image_file, **kwargs)
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/Format.py", line 168, in __init__
self.setup()
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/Format.py", line 178, in setup
self._start()
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/FormatNXmxEigerFilewriter.py", line 32, in _start
super()._start()
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/format/FormatNXmx.py", line 75, in _start
self._detector_model = dxtbx.nexus.get_dxtbx_detector(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/graeme/git/dials/modules/dxtbx/src/dxtbx/nexus/__init__.py", line 360, in get_dxtbx_detector
assert module.fast_pixel_direction.depends_on is not None
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: Please report this error at https://github.com/dials/dials/issues: |
|
I find it is interesting that this problem arises now since this is not a new software version as well as I can tell i.e. 2024.1 sounds like ~ 1 year old? |
|
That's right, the 2024.1 firmware update was the major one, we completed that ~1y ago. This was supposed to be a minor update to the Simplon API, and I don't know if it is versioned / where to find that information. It's possible that I've failed to set some essential keys in the API, and that is why the header is seemingly different. I think we'll need input from Dectris to understand whether the @depends_on chain has been removed, or if it's user error. |
|
@spmeisburger Looking through the thread I unfortunately don't know which filewriter format your using. So here a general overview: The release 2024.1 introduced support for writing NXmx (also called Filewriter2). Since this mainly is a format change there are quite a few common attributes shared with Filewriter(1). Unfortunately, this led to the removal of the depends_on field (for slow/fast pixel directions) which wasn't intended. After getting the first reports, we released 2024.1.1 which restores the old behavior for filewriter(1). Using the following link you can download the update package for 2024.1.1 solving this issue for filewriter(1) files. As Graeme mentioned we will look into a process to prevent such things in the future on the side. |
Dectris' latest update to the filewriter & API added some additional keys for NXmx compliance (yay!), however the metadata in the header is now slightly different.
We did the update here at CHESS id7b2 yesterday, and when I tried importing (via fast_dp autoprocessing) I got the following traceback:
The AssertionError refers to this bit of code:
dxtbx/src/dxtbx/nexus/__init__.py
Line 360 in 8f8574d
Note that, although I'm using a somewhat old DIALS version (3.14), the latest one makes the same assumption that module.fast_pixel_direction.depends_on is not None.
In the updated API, my master file has:
/entry/instrument/detector/module/fast_pixel_direction/@depends_on='.'
Before updating the API, the master file had:
/entry/instrument/detector/module/fast_pixel_direction/@depends_on='/entry/instrument/detector/transformations/translation'
It looks like
nxmxis attempting to determine the detector orientation from the @depends_on chain. However, the detector geometry is actually specified in/entry/instrument/detector/geometry, and the link to the "translation" of the detector is irrelevant to the fast/slow axis coordinate system, I believe. So it makes sense to me why Dectris would have removed it.In any case, to get things working here, I did a simple hack to restore the
depends_onlink in our custom CHESS dxtbx format:https://github.com/FlexXBeamline/dials-extensions/blob/85ea42fff45311b5176643cf4964cfdcafc3d1ef/dials_extensions/FormatNXmxEigerFilewriterCHESS.py#L53-L59
Longer-term, it might be a good idea to harmonize the detector geometry specification in the nxmx package with what Dectris is producing. We might be the first beamline to get this update, I'm not sure, but I assume others will encounter the issue as well.
The text was updated successfully, but these errors were encountered: