---
name: DIALS core meeting 2021-05-06
tags: core meeting
---

# DIALS core meeting 2021-05-06

[](https://hackmd.io/hoBqT2G3RuutbcyCFBsHJA)

## Previous Actions

* [ ] MG: organise a [typing intro lecture](https://dials.github.io/kb/core/20200917#typing-mypy), likely to happen in June.
* [ ] ND to set up a [`cbflib_adaptbx` pull request](https://dials.github.io/kb/core/20201125#cbflib_adaptbx-dependency) for MAR detector fixes. Verify with `labelit_regression`.
* [ ] [dx2 proposal](https://dials.github.io/kb/core/20210128#dx2-proposal)
* [ ] MG to put in a PR to create a proposal in the proposal-space
* [ ] MG+ND: Come up with a proposal to [move away from all code being in header files](https://dials.github.io/kb/core/20201001#overall-architecture-discussion) and consolidate into a single library
* [ ] [Move to `hdf5plugin`](https://dials.github.io/kb/core/20210225#change-hdf5-plugin-library)
* [x] MG to [come up with a set of spells](https://dials.github.io/kb/core/20210225#change-hdf5-plugin-library) to use `hdf5plugin`

       $ conda install hdf5plugin mamba
       $ mamba remove hdf5-external-filter-plugins{,-bitshuffle,-bzip2,-lz4}

* [x] GW to [run a performance comparison](https://dials.github.io/kb/core/20210225#change-hdf5-plugin-library)
* Provided an [initial version of performance comparison code](https://gist.github.com/ndevenish/bf03b390adddb33089a8c6a70e8f4936) -- we can run this with different plugins to assess whether there is any difference in run-time just paging through the data.
- [x] Cosym clustering code removal: ([discussion](https://dials.github.io/kb/core/20210422#discuss-changes-merged-into-cctbx-modify_cosym)) Merge [dials/dials#1647](https://github.com/dials/dials/pull/1647)
- [x] Has downstream had the correction commits?
- [ ] [cbflib conda-forge package](https://dials.github.io/kb/core/20210422#cbflib-conda-forge-package)
- [ ] ND: Try using old cbflib 0.9.6 (allowing regenerated pycbf) to see if it would be suitable for DIALS/dxtbx to release just the old version
- [x] AB: Create PR to DIALS for `xfel_regression` tests for further discussion/study. →[dials#1671](https://github.com/dials/dials/pull/1671), see below.

## Agenda

### `hdf5plugin` performance evaluation

HDF5 performance evaluation -- comparing `hdf5plugin` with `hdf5-external-plugins` package. simply read 3,600 images from an Eiger 16M data set:

import h5py
import sys
import time

with h5py.File(sys.argv[1], "r") as f:
    shape = f["/entry/data/data"].shape

    t0 = time.time()
    for j in range(shape[0]):
        image = f["/entry/data/data"][j]
    t1 = time.time()

    print(f"Reading {shape[0]} images took {(t1-t0):.2f}s")

Using `hdf5-external-plugins`:

$ time dials.python benchmark.py ../data/*nxs
Reading 3600 images took 270.23s

real    4m30.382s
user    2m46.693s
sys     1m43.610s

With `hdf5plugin`:

Added

import hdf5plugin

to imports then run:

$ time dials.python benchmark.py ../data/*nxs
Reading 3600 images took278.19s

real    4m38.331s
user    2m39.540s
sys     1m58.604s

8s of 280 or so more time (2.5%) → this may or may not be a thing we care about, but the performance is comparable. EOF.

**Outcome:** We will move to `hdf5plugin`, make a pull request and then review.

### Adding XFEL-regression to dials Azure tests ([dials#1671](https://github.com/dials/dials/pull/1671))
- MG has updated the Azure scripts for DIALS tests in [#1676](https://github.com/dials/dials/pull/1676).
- MG looked at using pre-built cctbx in tests.
- For dxtbx this works nicely. Relevant PR is [dxtbx#351](https://github.com/cctbx/dxtbx/pull/351), which is ready to be merged.
- Didn't want to spend considerable effort getting this to work for dials before having the dxtbx PR above signed off.
- Have timeout problems gone away?
- Have we resolved issue of protocols for merging PRs/making changes when tests fail?
- Tests will run in the PR build only, so not on every push

**Outcome:** Merge [dxtbx#351](https://github.com/cctbx/dxtbx/pull/351), then MG to look at the actual XFEL-regression pull request

### [cbflib conda-forge package](https://dials.github.io/kb/core/20201125#cbflib-conda-forge-package)
* Keeping tests passing on Debian downstream - this is an essential use case and new releases cannot cause debian changes in any way
* HB estimates graduate student project with approximately 3 months of effort required
* **Action: Nick to do testing:** Trying current release - regenerate pycbf but leave old sources; and see if it works
* we could just release 0.9.6 on conda-forge with some light patching over the top if this works

### [dxtbx#288](https://github.com/cctbx/dxtbx/pull/288) "Construct Experiments directly" post-merge outstanding issues

- [dxtbx#336](https://github.com/cctbx/dxtbx/issues/336) - DataBlockFactory broken for single images
- XFAIL test for this now put in: [dxtbx#337](https://github.com/cctbx/dxtbx/pull/337)

### Python 3.6 Support

- Releases are now built with 3.8
- Do we want to consider dropping support for 3.6?

We currently have an agreement in place in [dials#1327](https://github.com/dials/dials/issues/1327) to follow conda-forge which aim to follow NEP-29. This is less well defined than following NEP-29.

**Outcome:** Put in a proposal ("DC3") to follow NEP-29 directly, with a bit of additional allowance around releases, as in [dials#1327](https://github.com/dials/dials/issues/1327).

⇒ DC3 proposal written up in [dials/kb#5](https://github.com/dials/kb/pull/5)

### Updating pre-commits

We currently update the black pre-commit whenever a new version appears.
While this is currently a rare occurrence, it is annoying for one commit (due to the installation time), and possibly requires updating other packages in the environment (eg. `pre-commit` itself).
Q: Should we stop automatically updating pre-commits and turn those updates into conscious moves?

**Outcome:** Yes. ND to remove.

### Next meeting
Thursday, May 20th, 4pm UK (BST), 8am PDT.