Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NXmx general writer #615

Merged
merged 39 commits into from
Aug 14, 2023
Merged

NXmx general writer #615

merged 39 commits into from
Aug 14, 2023

Conversation

phyy-nx
Copy link
Contributor

@phyy-nx phyy-nx commented Mar 21, 2023

This adds a general NXmx writer to dxtbx

Implemented along the lines of the existing cbf_writer that is used by the image averager, it has these important features:

  • Multi panel, hierarchical detector support. This combines homogenous coordinate transformations implemented for the cbf_writer with the NeXus writing code in the SwissFEL writer to read general dxtbx detector models and write them in NeXus format.
  • Handles the general spectra case, including 1D and 2D spectra, single shot and multishot spectra, and testing for variants, where the wavelength is not the same as the spectrum weighted wavelength. Tested using SwissFEL data with spectra.
  • Handles appending data to existing file by using special h5py syntax for noting a dataset has no maximum size.
  • Handles using external untrusted pixels masks from DIALS and adding them to NeXus

What is missing:

  • Scans
  • Gonios

The plan is to add these in an upcoming DIALS code camp :)

Here's a couple of examples of running it. First, this one eats 10 cbfs files and makes a single NeXus file:

libtbx.python `libtbx.find_in_repositories dxtbx`/src/dxtbx/format/nxmx_writer.py ../thaumatin/data/th_8_2_000*.cbf output_file=ten_thau.h5 instrument_name="dummy" source_name="dummy" start_time="dummy" end_time_estimated="dummy" sample_name="dummy"

Note that there are some required NeXus keys that can't be read from the dxtbx object models. Those have to be provided somehow so I've made the API require them.

Second example. In this one I am trying to be more careful about the required NeXus metadata. This example is on a SwissFEL master with spectra, so the result combines the master and its subfiles into one file.

libtbx.python `libtbx.find_in_repositories dxtbx`/src/dxtbx/format/nxmx_writer.py ../run_000795.JF07T32V01_raw_master_new.h5 output_file=run_795.h5 instrument_name="SwissFEL ARAMIS BEAMLINE ESB" instrument_short_name="ESB" source_name="SwissFEL ARAMIS" source_short_name="SwissFEL ARAMIS" start_time="2022-09-01T00:00:00.000" end_time_estimated="2022-09-01T00:05:00.000Z" sample_name="Sample1"

I think there's some room for improvement:

  • I think the bitdepth of the output is too high (int32 or float64). I think it should be able to autodetect the useful bit depth, then downshift as needed.
  • I also think we can compress the data.

@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Merging #615 (4f1501e) into main (eda7169) will decrease coverage by 0.98%.
The diff coverage is 0.24%.

❗ Current head 4f1501e differs from pull request most recent head 273b01a. Consider uploading reports for the commit 273b01a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #615      +/-   ##
==========================================
- Coverage   39.08%   38.11%   -0.98%     
==========================================
  Files         181      182       +1     
  Lines       15864    16268     +404     
  Branches     3066     3160      +94     
==========================================
  Hits         6201     6201              
- Misses       9077     9481     +404     
  Partials      586      586              

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Mar 25, 2023

Ok, this is ready for others to look at. Notes:

  1. This PR includes NXmx: handle multidimensional arrays #612 so that should probably go in first
  2. There are changes here to imageset.py, and nexus/__init.py that I found while working on the writer
  3. I think the fixed rotation rotation matrix in goniometers is not supported here, which is worthy of review
  4. I think the s0 vector isn't supported either

I'd like to take a look at 3 and 4 before merging, but they aren't stoppers I think.

@phyy-nx phyy-nx marked this pull request as ready for review March 25, 2023 00:13
@phyy-nx phyy-nx requested a review from rjgildea March 25, 2023 00:13
phyy-nx and others added 26 commits July 26, 2023 09:05
Data in NeXus can be 3 or 4 dimensional.
3D: Nimages by slow by fast
4D: Nimages by Nmodules by slow fast

Slice image_size and reshape the raw_data in these cases
- Remove hardcoded SwissFEL terms and add documention
- Add and use validation function for required items that can't be found in the dxtbx object model
This way assumes adding them one at a time.  For spectra, this usecase assumes only one set of energy channels and no variants.
Supports single and multi-axes goniometers

Co-authored-by: Richard Gildea <richard.gildea@diamond.ac.uk>
Combine them using the offset attribute
Add the fast/slow origin offsets for a flat hierarchy detector.  Usually zero for Dectris data, but the cbf_writer encodes an offset here for leaf transformations
One reads a JF16M detector and the other 10 cbf files.  Currently just checks the detector model
…riting a serial crystallographic dataset to NXmx
@phyy-nx phyy-nx requested review from graeme-winter and removed request for rjgildea August 4, 2023 00:36
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Aug 4, 2023

Ok the tests should pass now. This ready for re-review! Again, depends on #612.

@phyy-nx
Copy link
Contributor Author

phyy-nx commented Aug 14, 2023

@ndevenish looks ready? :)

@ndevenish
Copy link
Collaborator

Yeah, I think so. The release is currently running through CI, which means the branch-off is chosen, so it'll be "official" next release, barring build failures.

@ndevenish ndevenish merged commit 0554ef3 into main Aug 14, 2023
20 checks passed
@ndevenish ndevenish deleted the nxmx_writer branch August 14, 2023 18:26
@phyy-nx
Copy link
Contributor Author

phyy-nx commented Aug 14, 2023

Yay!

toastisme pushed a commit to toastisme/dxtbx that referenced this pull request Jul 18, 2024
Writes any data that dxtbx can import, out to NeXus NXmx .
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants