---
name: DIALS core meeting 2020-11-25
tags: core meeting
---

# DIALS core meeting 2020-11-25

## Previous Actions

* [ ] MG+ND: write a proposal for ['overall architecture discussion'](https://dials.github.io/kb/core/20200903) and ['new installer'](https://dials.github.io/kb/core/20200903)
* [ ] MG+ND: Come up with a proposal to [move away from all code being in header files](https://dials.github.io/kb/core/20201001) and consolidate into a single library
* [ ] ND: update `CONTRIBUTING` in a (separate) pull request to include a soft-imposition of [stable master](https://dials.github.io/kb/core/20200903), [dials#1353](https://github.com/dials/dials/issues/1353).
* [ ] MG: organise a [typing intro lecture](https://dials.github.io/kb/core/20200917).
* [x] GW: [move `cbflib_adaptbx`'s `compress` and `uncompress` functions](https://dials.github.io/kb/core/20201001) into dxtbx in a PR.
* broke building on Mac completely, and looked like it broke building on Linux but may have been a coincidence. ND looking at it.
* Now merged as [dxtbx#238](https://github.com/cctbx/dxtbx/pull/238)
* [x] MG to [set up dxtbx/dials Azure builds against cctbx conda-forge package](https://dials.github.io/kb/core/20201015)
* [x] MG to finish up [DC1](https://hackmd.io/@dials/B11SXgTND) pull request for xia2
* [x] DP to merge [dxtbx#210](https://github.com/cctbx/dxtbx/pull/210)

## Agenda

### dxtbx/dials Azure builds against cctbx conda-forge package

Ready in [dxtbx#262](https://github.com/cctbx/dxtbx/pull/262), based on cctbx release 2020.10.
This adds a single linux build in addition to the currently 2 (uncached) linux and macos builds.

Build times are significantly reduced: about 2 minutes environment setup, 2 minutes build time, 4 minutes test time, for 8 minutes in total. An uncached linux build otherwise takes >30 minutes.

Go / no go decision?

* cctbx can do approximately weekly point releases to include bugfixes
* Let's do this.

### xia2 DC1

Ready in [xia2#528](https://github.com/xia2/xia2/pull/528).

This changes the repository to source layout (files that used to be in `/` are moved to `/src/xia2`).

Full description of changes in [xia2#528#comment](https://github.com/xia2/xia2/pull/528#issuecomment-733792471), including the release versioning process.

Go / no go decision?

* ND to review
* followed by 'let's do this'.

### reduce package
Concerns on impact of the new `reduce` package ([cctbx#567](https://github.com/cctbx/cctbx_project/pull/567)) on DIALS releases

BKP: license file issue is acknowledged and being worked on. `reduce` code is older than cctbx.

MG: `CCIBuilder` is possibly not the best place to add this?

BKP: Will review

ASB: We are using `DIALSBuilder` and base builds.

Outcome: We should be fine whether or not reduce is distributed via the cctbx conda-forge package.

### cbflib conda-forge package
Have merged in ND's working branch to repo for pull: https://github.com/conda-forge/staged-recipes/pull/12742. Also built as manual [feedstock](https://github.com/ndevenish/cbflib-feedstock), so available for install on conda-channel `-c ndevenish cbflib`. Limitations of approach:
- Windows not working yes, various compile errors including C99-related
- Fortran build seems to error on macOS - hdf5 only builds static libs?
- Builds off master by replacing `CMakeLists.txt`, but currently pointed to [yayahjb/cbflib#19](https://github.com/yayahjb/cbflib/pull/19) for python 3 behavioural issues

Actions:
* ND to set up an agenda for a code-review meeting
* ASB to set up a meeting with HJB, BKP, ASB, ND
* ASB to explore what should happen to cbflib maintenance in the future

### `cbflib_adaptbx` dependency
ND has a few changes to fix the MAR detector without `cbflib_adaptbx` dependency [cctbx/dxtbx#264](https://github.com/cctbx/dxtbx/issues/264)

Actions:
* ND to set up a pull request from that
* ASB: Make sure to run labelit_regression tests on that

### dials-data-files
Aaron would like to add a set of files:
[Add first 4 frames of cxidb entry 33 from Takanori Nakane](https://github.com/dials/data-files/pull/22)

MG merged this PR.

The upcoming PR to dials-data is welcome. This might not work directly because of Travis CI policy changes. MG will sort it if there are technical issues.

### Definition of `trusted_range`
*not discussed in this meeting*

Discuss PR [Definition of `trusted_range` (dxtbx#182)](https://github.com/cctbx/dxtbx/issues/182)

### `image_range` vs. `array_range` baked in off-by-one errors
*not discussed in this meeting*

Discuss [`image_range` vs. `array_range` baked in off-by-one errors (dxtbx#186)](https://github.com/cctbx/dxtbx/issues/186)
* DW made a proposal and started work on that, but it proved messy
* There is a new, simpler, proposal but this version kills off the `image_range` concept rather than `array_range`
* Discuss, again, what we want here and whether the new proposal is useful

### dxtbx json/msgpack performance
*not discussed in this meeting*

ASB reports json being problematic when importing e.g. 50,000 experiments, as everything is stored in a single file, so everything needs to be read at once.

* GW: cf. [Load before heat death (dxtbx#118)](https://github.com/cctbx/dxtbx/pull/118). At the core of it are bad design choices in dxtbx.
* Future discussion topics:
1. treatment of large serial datasets
2. using an HDF5 backend for reflection files
3. [dials#1407](https://github.com/dials/dials/issues/1407) specific proposal
* ASB: I want `check_format` to go away.

Previous discussion outcomes:

* ASB:
* General consensus that this is needed.
* Make models and store them
* Load them in parallel with minimal memory requirements
* Version all models and rewind them
* This needs a lot of thought and at this time we don't have agreement here.
* Not need to copy data like shoeboxes when they aren't being changed, operate by reference

### Remove `tntbx` dependency

Appears not to be used anywhere outside of `cctbx_project/xfel`, but `xfel` does not declare it as a dependency. `xfel` only uses the [`svd()`](https://github.com/dials/tntbx/blob/master/tntbx/__init__.py#L7) function. Maybe change xfel to use [`numpy.linalg.svd`](https://numpy.org/doc/stable/reference/generated/numpy.linalg.svd.html) instead and drop `tntbx` entirely?

Peter Zwart: My favorite SVD tools lives in [`scipy.sparse.linalg.svds`](https://docs.scipy.org/doc/scipy/reference/generated/scipy.sparse.linalg.svds.html#scipy.sparse.linalg.svds). This routine allows calculation of the top K vectors in an SVD, it is a massive time saver if you know the rank of the matrix. The code works for dense as well as sparse matrices.

ASB: This seems to be obsolete code. I think if you dropped the dependency from DIALS it wouldn't hurt us

Action:
* [x] remove `tntbx` from bootstrap -> [dials#1507](https://github.com/dials/dials/pull/1507)

### renaming `master` branch → `main`
*not discussed in this meeting*

* Nothing much will happen on this any time soon - either way we'll [wait for GitHub support on this one](https://github.com/github/renaming)
* General assent that this is useful
* since 1st of October new GitHub repositories have `main` as the default
* **proposed outcome**: Agree to migrate to `main`. Turn this into dxtbx/dials/xia2 issues and move once GitHub provides a migration path. Use xia2 as a test-bed and move dials and dxtbx two weeks later.

### DIALS documentation build
*not discussed in this meeting*

Should the DIALS documentation build outside a cctbx environment?

* Currently you can only build the DIALS documentation inside a cctbx environment. This means a remote documentation build needs to build cctbx first, so we can't easily run the documentation build on every commit. When things break they are difficult to fix. We can't use readthedocs.
* MG: I believe the main obstacles are three cctbx Sphinx plugins that may need to be extracted or removed from DIALS documentation, and some phil parsing logic.
* MG: the cctbx conda-forge package may be useful here

### move the active `dxtbx` repository into the DIALS organisation
*not discussed in this meeting*

Having the main repository in cctbx_project has a few distinct disadvantages. The most annoying is the [eternal fight against the lockdown bot](https://github.com/dials/dxtbx/pulls?q=is%3Apr+is%3Aclosed+-label%3Aupcoming-release+), but we also don't get LGTM output.

* **proposed outcome**: move the active `dxtbx` repository into the DIALS organisation. Do it in such a way that the impact on existing installations, issues, pull requests is minimal.

### dials.index: create new experiments
*not discussed in this meeting*

dials.index should (or should not) create new experiments with crystals in rather than modifying in place existing experiments [as proposed in this issue](https://github.com/dials/dials/issues/1029)

### Next meeting
*Wednesday*, December *9th*, 4pm UK time, 8am PDT.

Ring-fenced to discuss pull requests with the word 'range' and/or 'lazy' in the subject, and performance issues baked into dxtbx.