cctbx / dxtbx

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

h5py=3.1 upgrade causes test_FormatHDF5EigerNearlyNexusSPring8 failure #267

Closed rjgildea closed 3 years ago

rjgildea commented 3 years ago

With the previous h5py=2.10.0 this test passes as expected:

$ dials.python -c "import h5py; print(h5py.__version__)"
2.10.0
$ pytest tests/format/test_FormatHDF5EigerNearlyNexusSPring8.py --regression -vs
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.8.6, pytest-6.1.2, py-1.9.0, pluggy-0.13.1 -- /dls/science/users/hko55533/software/cctbx/build/../conda_base/bin/python3.8
cachedir: .pytest_cache
rootdir: /dls/science/users/hko55533/software/cctbx/modules/dxtbx, configfile: pytest.ini
plugins: dials-data-2.1.30, forked-1.2.0, mock-1.13.0, xdist-2.1.0, cov-2.10.1
collected 1 item                                                                                                                                                                                                  

tests/format/test_FormatHDF5EigerNearlyNexusSPring8.py::test_spring8_ccp4_2018_zenodo_1443110_data03 PASSED

After upgrading h5py to 3.1:

$ conda install h5py=3.1
Collecting package metadata (current_repodata.json): done
Solving environment: done

## Package Plan ##

  environment location: /dls/science/users/hko55533/software/cctbx/conda_base

  added / updated specs:
    - h5py=3.1

The following packages will be UPDATED:

  h5py                        2.10.0-nompi_py38h7442b35_105 --> 3.1.0-nompi_py38hafa665b_100

this test now fails with the following error:


    def test_spring8_ccp4_2018_zenodo_1443110_data03():
        # https://zenodo.org/record/1443110#.XD8bD5ynzmE
        master_h5 = "/dls/science/groups/scisoft/DIALS/zenodo/1443110/ccp4school2018_bl41xu/05/data03/data03_master.h5"
        assert FormatHDF5EigerNearlyNexusSPring8.understand(master_h5)

>       expts = ExperimentListFactory.from_filenames([master_h5])

tests/format/test_FormatHDF5EigerNearlyNexusSPring8.py:27: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
model/experiment_list.py:525: in from_filenames
    for db in DataBlockFactory.from_filenames(
datablock.py:1489: in from_filenames
    importer = DataBlockFilenameImporter(
datablock.py:934: in __init__
    imageset = self._create_single_file_imageset(
datablock.py:1153: in _create_single_file_imageset
    return format_class.get_imageset(abspath(filename), format_kwargs=format_kwargs)
format/FormatMultiImage.py:147: in get_imageset
    format_instance = cls(filenames[0], **format_kwargs)
format/FormatHDF5.py:15: in __init__
    Format.__init__(self, image_file, **kwargs)
format/Format.py:137: in __init__
    self.setup()
format/Format.py:147: in setup
    self._start()
format/FormatHDF5EigerNearlyNexusSPring8.py:26: in _start
    super(FormatHDF5EigerNearlyNexusSPring8, self)._start()
format/FormatHDF5EigerNearlyNexus.py:366: in _start
    self._goniometer_model = GoniometerFactory(sample).model
format/nexus.py:1118: in __init__
    axes, angles, axis_names, scan_axis = construct_axes(
format/nexus.py:339: in construct_axes
    visitor.visit(nx_file, item)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <dxtbx.format.nexus.construct_axes.<locals>.Visitor object at 0x7f0b8cebfeb0>, nx_file = <HDF5 file "tmp_master_0ab6ed0a3dea11eb82d5989096dc1b01.nxs" (mode r+)>
depends_on = '/entry/sample/transformations/omega'

    def visit(self, nx_file, depends_on):
        item = nx_file[depends_on]
        value = item[()]
        units = item.attrs["units"]
        ttype = item.attrs["transformation_type"]
        vector = [float(v) for v in item.attrs["vector"]]
        if ttype == numpy.string_("translation"):
            return
        elif ttype == numpy.string_("rotation"):
            if hasattr(value, "__iter__") and len(value):
                value = value[0]
            if units == numpy.string_("rad"):
                value *= 180 / math.pi
            elif units not in [
                numpy.string_("deg"),
                numpy.string_("degree"),
                numpy.string_("degrees"),
            ]:
                raise RuntimeError("Invalid units: %s" % units)

            # is the axis moving? Check the values for this axis
            v = item[()]
            if min(v) < max(v):
                is_scan_axis = True
            else:
                is_scan_axis = False

            # Is different coordinate system called mcstas
            # Rotate 180 about up if memory serves
            axis_name = item.name.split("/")[-1]
            self._axes.append(vector)
            self._angles.append(float(value))
            self._axis_names.append(str(axis_name))
            self._is_scan_axis.append(is_scan_axis)

        else:
>           raise RuntimeError("Unknown transformation_type: %s" % ttype)
E           RuntimeError: Unknown transformation_type: rotation

format/nexus.py:305: RuntimeError
graeme-winter commented 3 years ago

Does https://github.com/cctbx/dxtbx/pull/263 fix it?

graeme-winter commented 3 years ago

Also humbly & respectfully note that this was my fault, should have run more comprehensive tests before putting the 3.1 PR change in.

rjgildea commented 3 years ago

Checking out the branch in #263 results in a different error:

    def test_spring8_ccp4_2018_zenodo_1443110_data03():
        # https://zenodo.org/record/1443110#.XD8bD5ynzmE
        master_h5 = "/dls/science/groups/scisoft/DIALS/zenodo/1443110/ccp4school2018_bl41xu/05/data03/data03_master.h5"
        assert FormatHDF5EigerNearlyNexusSPring8.understand(master_h5)

>       expts = ExperimentListFactory.from_filenames([master_h5])

tests/format/test_FormatHDF5EigerNearlyNexusSPring8.py:27: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
model/experiment_list.py:525: in from_filenames
    for db in DataBlockFactory.from_filenames(
datablock.py:1489: in from_filenames
    importer = DataBlockFilenameImporter(
datablock.py:934: in __init__
    imageset = self._create_single_file_imageset(
datablock.py:1153: in _create_single_file_imageset
    return format_class.get_imageset(abspath(filename), format_kwargs=format_kwargs)
format/FormatMultiImage.py:147: in get_imageset
    format_instance = cls(filenames[0], **format_kwargs)
format/FormatHDF5.py:15: in __init__
    Format.__init__(self, image_file, **kwargs)
format/Format.py:137: in __init__
    self.setup()
format/Format.py:147: in setup
    self._start()
format/FormatHDF5EigerNearlyNexusSPring8.py:26: in _start
    super(FormatHDF5EigerNearlyNexusSPring8, self)._start()
format/FormatHDF5EigerNearlyNexus.py:366: in _start
    self._goniometer_model = GoniometerFactory(sample).model
format/nexus.py:1118: in __init__
    axes, angles, axis_names, scan_axis = construct_axes(
format/nexus.py:339: in construct_axes
    visitor.visit(nx_file, item)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <dxtbx.format.nexus.construct_axes.<locals>.Visitor object at 0x7f7d4dc3c3a0>, nx_file = <HDF5 file "tmp_master_3dddf71e3def11ebba47989096dc1b01.nxs" (mode r+)>
depends_on = '/entry/sample/transformations/omega'

    def visit(self, nx_file, depends_on):
        item = nx_file[depends_on]
        value = item[()]
        units = item.attrs["units"]
        ttype = item.attrs["transformation_type"]
        vector = [float(v) for v in item.attrs["vector"]]
        if numpy.string_(ttype) == numpy.string_("translation"):
            return
        elif numpy.string_(ttype) == numpy.string_("rotation"):
            if hasattr(value, "__iter__") and len(value):
                value = value[0]
            if numpy.string_(units) == numpy.string_("rad"):
                value *= 180 / math.pi
            elif units not in [
                numpy.string_("deg"),
                numpy.string_("degree"),
                numpy.string_("degrees"),
            ]:
>               raise RuntimeError("Invalid units: %s" % units)
E               RuntimeError: Invalid units: degree

format/nexus.py:287: RuntimeError
ndevenish commented 3 years ago

Also humbly & respectfully note that this was my fault, should have run more comprehensive tests before putting the 3.1 PR change in.

No, I think what you did was reasonable. This test only works at diamond https://github.com/cctbx/dxtbx/blob/6370c7a410fb6a80aaeb0e0e89a74337e2204c07/tests/format/test_FormatHDF5EigerNearlyNexusSPring8.py#L22-L24

rjgildea commented 3 years ago

I think this test should be able to use dials.data now: https://github.com/dials/data/issues/11 https://github.com/dials/data/pull/192

ndevenish commented 3 years ago

Okay, so #263 fixes this error now, but not yet https://github.com/dials/dials/issues/1523