DIALS core meeting 2021-05-06

Previous Actions

• MG: organise a typing intro lecture, likely to happen in June.
• ND to set up a cbflib_adaptbx pull request for MAR detector fixes. Verify with labelit_regression.
• 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 and consolidate into a single library
• Move to hdf5plugin
  • MG to come up with a set of spells to use hdf5plugin

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

  • GW to run a performance comparison
    • Provided an initial version of performance comparison code – we can run this with different plugins to assess whether there is any difference in run-time just paging through the data.
• Cosym clustering code removal: (discussion) Merge dials/dials#1647
  • Has downstream had the correction commits?
• 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
• AB: Create PR to DIALS for xfel_regression tests for further discussion/study. →dials#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)

• MG has updated the Azure scripts for DIALS tests in #1676.
• MG looked at using pre-built cctbx in tests.
  • For dxtbx this works nicely. Relevant PR is dxtbx#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, then MG to look at the actual XFEL-regression pull request

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 “Construct Experiments directly” post-merge outstanding issues

• dxtbx#336 - DataBlockFactory broken for single images
• XFAIL test for this now put in: dxtbx#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 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.

⇒ DC3 proposal written up in dials/kb#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.