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

# DIALS core meeting 2020-11-12

[](https://hackmd.io/vg6CxoYUSseU7ylz9-v40w)

## 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)
* [x] MG+ND: [Implement](https://dials.github.io/kb/core/20201001) [DC1 -- per-module `setup.py`](https://hackmd.io/@dials/B11SXgTND) in a pull request -- see below
* [ ] 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).
* [ ] 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.
* ASB to ask BKP whether fundamentally `scons` could run `cmake` under the hood for a module. Answer: You can always write a python script running `cmake`.
* [x] JBE: to [set up meeting with Andrey/Randy/Peter with DIALS-E folks](https://dials.github.io/kb/core/20201015) at a suitable date & time.
* [x] ASB to [organise a meeting on the image viewer](https://dials.github.io/kb/core/20201015) - coming up next week.
* [ ] MG to [set up dxtbx/dials Azure builds against cctbx conda-forge package](https://dials.github.io/kb/core/20201015)
* currently blocked by [issue #4](https://github.com/conda-forge/cctbx-base-feedstock/issues/4)

## Agenda

### [DC1 -- per-module `setup.py`](https://hackmd.io/@dials/B11SXgTND)

As [previously agreed](https://dials.github.io/kb/core/20201001) we now have a pull request demonstrating DC1 for xia2 in https://github.com/xia2/xia2/pull/528.

Open questions that need deciding:
* level of automagical behaviour of a `command_line` equivalent directory
* Proposal: No automagical behaviour, move scripts from `command_line/` to `[src/xia2/]cli/` to indicate a clear break between philosophies.
* version numbering
* Proposal: xia2 to track DIALS version numbers. `master` branch immediately goes to 3.3.dev, future releases become 3.3.0, 3.3.1, etc. Each patch release gets an individual tag. Use bump2version to update entries and create tags. We lose the git tag and branch version suffix we have at the moment.
* Outcome:
* For xia2 this is fine
* There may be more discussion on the impact on DIALS going forward, which will be discussed and decided in a future meeting here. We will invite more people when we have that conversation.

### cctbx conda-forge package
We want to evaluate moving the dxtbx/DIALS Azure builds to build on top of the [cctbx conda-forge package](https://github.com/conda-forge/cctbx-base-feedstock).

Consequences: We will get faster builds and clearer API boundaries and versioning, however it also means that we start treating cctbx as a software package. This restricts dxtbx/DIALS code to only use functionality that is part of the relevant cctbx release. From time to time, when cctbx releases new versions, we would update our dependencies, which would then allow the use of new cctbx features.

* ASB: Was discussed at the CCI-all meeting, and the outcome was: this is fine. I need to be able to continue using cctbx as a development installation. Apart from that it is fine to treat cctbx as a versioned software package, ie. run Azure against the cctbx-base conda-forge package.
* The drawbacks noted above will thus apply.

RG raised concerns after the last meeting, so left on agenda for further discussion.
* [cctbx#560](https://github.com/cctbx/cctbx_project/pull/560)
* We would need to be able to make cctbx patch releases. This is because
* the DIALS master branch relies on the cctbx release version
* when a DIALS release is branched off that will rely on the latest cctbx release version
* we are unlikely to change cctbx release versions during a DIALS release cycle
* therefore we have to support the cctbx release version (from branch day) throughout the DIALS release cycle (~2.5 months avg)
* MG: In the worst case this would mean a C++ change (so no monkeypatching) that was required more than one month after the cctbx release (so that another monthly release has happened in the meantime), therefore a conda-forge patch release would need to happen on a branch. This would need a few roundtrips in communication as
* BKP would need to create a branch in the conda-forge feedstock
* we would need to put the patches in as a pull request against that branch
* BKP would need to merge that PR
* ASB: I am sure we could get BKP's support on this when needed

### `dxtbx` → `xfel` dependencies
*not discussed in this meeting*

This discussion has now moved into [dxtbx#239](https://github.com/cctbx/dxtbx/issues/239).

### NIAC Nexus
*not discussed in this meeting*

This discussion has now moved into [#1474](https://github.com/dials/dials/issues/1474).

### Pull request *When file indices are provided,...*

ASB: This is [dxtbx#210](https://github.com/cctbx/dxtbx/pull/210). The `FormatSaclaHDF5` class isn't lazy, which led to that problem. Will fix this in a separate PR, but the PR above is still a good PR independently of that.

In discussion we identified a new potential issue. We need to ensure during deserialisation this loop isn't hit for classes that don't have scans and goniometers.

**Outcome:** DP to merge.

### 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
*not discussed in this meeting*

Appears not to be used anywhere outside of `cctbx_project/xfel`, but `xfel` does not declared is 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.

### 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
* from 1st of October new GitHub repositories will 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)

### Compilers

ASB can invite JB to discuss cross-compiling to HPC architectures

### Next meeting
November 25th, 4pm UK time, 8am PDT.