DIALS core meeting 2020-10-01

Previous Actions

• MG+ND: write a proposal for ‘overall architecture discussion’ and ‘new installer’
  • first outcome to be discussed below
• ND: update CONTRIBUTING in a (separate) pull request to include a soft-imposition of stable master, dials#1353.
  • Was waiting for dials#1395 which has not received any negative comments - so let’s merge that.
• MG: organise a typing intro lecture.

Agenda

Overall architecture discussion

Discuss proposal DC1 – per-module setup.py

• GW: Can we move away from all code being in header files and can we have a single library?
  • Discussed. Sounds doable, but should be a separate proposal. AP: MG+ND to make such a proposal.
• ND notes that this removes the need for the current entry point hack. We should add that to the proposal.

Outcome: AP: MG+ND to make this happen in a PR

cbflib_adaptbx dependency

We are only referencing compress and uncompress functions, and cbflib_adaptbx has a MSVC-ancient-dependency. We could copy the functions into dxtbx_ext and remove the dependency.

• GW: This code is too inflexible for current needs, so may need to rewrite it.
• MG: I think these two functions do not even go into cbflib. They are used in a test in iotbx/detectors
• GW: I’m not proposing taking the functions out of cbflib_adaptbx
• ASB: You may want to think about taking it out of cbflib_adaptbx. Discussions with Herbert about dissolving cbflib_adaptbx are ongoing. Before removing though need to check for eg. labelit dependencies.

AP: GW to move the function into dxtbx in a PR

Remove clipper dependency (#1437)

Doesn’t appear to be used anywhere, so we basically just redistribute the source code.

• Not used by anything known. If it was then we ought to find out.

Outcome: Merge PR and remove

cctbx conda-forge package

We may want to evaluate moving the dxtbx/DIALS Azure builds to use the now available cctbx conda-forge package, once issue #4 is resolved, which prevents the package being used as a build platform.

• ABS: That would be rad.
• We haven’t appreciated the fundamental perspective shift yet though.
• ASB: Will bring this up the next CCI-all meeting.
• GW: We should bring our heads round the idea that all different components become different conda packages.

Benefits

• Faster builds
• Making use of clearer API boundaries and versioning

Drawbacks

• API boundaries and versioning
  • In other words: We would 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.

Outcome: Discuss next time, with ASB reporting from the CCI-all meeting.

dxtbx → xfel dependencies

At some point we made a somewhat conscious decision that dxtbx should not depend on dials. While setting up some Azure testing of dxtbx (#226) without dials/xia2 and their dependencies present some test failures were flagged because dxtbx has dependencies into xfel.

How should we deal with this? A few options:

• Disable the code so it stays in dxtbx but is not tested/missing dependencies do not trigger errors?
• Cleanest option would be to move xfel-dependent format classes into the xfel package
• Eliminate the dependencies

Discussion:

• GW: I have the opinion that we shouldn’t have circular dependencies. Ever.
• DGW: has electron-diffraction format classes outside of dxtbx. Would love to put SMV classes into a separate box as they are too promiscuous.
• ASB: having format classes stashed together is an easy mental model

Outcomes:

• ASB needs to consider the future of the xfel module
• In principle the format classes could go into xfel
• The commit linked above can go in (will be merged as part of dxtbx#226)
• Will discuss next time

Pull request When file indices are provided,…

ASB: This is dxtbx#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.

Definition of trusted_range

was not discussed today

Discuss Definition of trusted_range (dxtbx#182)

image_range vs. array_range baked in off-by-one errors

was not discussed today

Discuss image_range vs. array_range baked in off-by-one errors (dxtbx#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

DIALS documentation build

was not discussed today

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

dxtbx json/msgpack performance

was not discussed today

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). 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 specific proposal
• ASB: I want check_format to go away.

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

was not discussed today

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() function. Maybe change xfel to use numpy.linalg.svd instead and drop tntbx entirely?

renaming master branch → main

was not discussed today

• Nothing much will happen on this any time soon - either way we’ll wait for GitHub support on this one
• 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/dxtbx two weeks later.

Next meeting

October 15th, 4pm UK time, 8am PDT.