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

2d detector projection #224

Merged
merged 9 commits into from
Oct 16, 2020
Merged

2d detector projection #224

merged 9 commits into from
Oct 16, 2020

Conversation

dagewa
Copy link
Member

@dagewa dagewa commented Sep 29, 2020

This is an attempt to address #17, at least for the common case of (almost) co-planar detectors. A new function, project_2d is added which calculates an orthographic projection of the detector onto a 2D coordinate system. This differs from the dials.image_viewer in that the projection is not done in the laboratory frame, but in a "best fit" frame calculated by fitting a plane to the panels and aligning X and Y with the consensus fast, slow axes of the detector. As a result, bulk rotations of the detector up to 45° do not affect the 2D representation. A new test is added to check that.

The function has been added to detector_helpers.py for testing. To facilitate that, a new mode has also been added to dxtbx.plot_detector_models. If the parameters orthographic=True project_onto=image_plane are passed to this program it will produce a plot using the project_2d function.

The attached panel-beating.zip provides data to demonstrate, using a CS-PAD image, which is the most complicated co-planar geometry we currently support. Displaying imported.expt in dials.image_viewer and with the new mode to dxtbx.plot_detector_models demonstrates how the two viewers operate in a "normal" situation.

imported

If now the whole detector is rotated about an arbitrary axis by 20 degrees then dials.image_viewer displays the "jaunty angle" in the laboratory frame, whereas the new function is unaffected

rotated_detector

Finally, to demonstrate the choice of a consensus X, Y frame for display, we do a real nasty on the CS-PAD and break a sensor in half and leave it hanging by a thread. The dials.image_viewer displays this vandalism in the lab frame, whereas the new function projects it onto the best fit frame.

rotated_panel

There is not a perfect alignment of the frames shown by dxtbx.plot_detector_models rotated_detector.expt rotated_panel.expt orthographic=True project_onto=image_plane, so I can see how it would continue to be better to do metrology refinement using the lab frame projection.

Screenshot from 2020-09-29 11-42-06

But perhaps this would provide a more natural view for standard image viewer tasks? If in future this function is deemed suitable as an option for display in an image viewer (see dials/dials#1358) then it could in principle be moved to the detector model itself, so that it can easily be overridden for special cases like the I23 detector, where the assumption of a single best-fit plane breaks down.

Project a detector model into 2D using the best fit plane to the
panels and aligning to the panel axes
If orthographic=True and project_onto=image_plane, use project_2d
to do the projection
@phyy-nx
Copy link
Contributor

phyy-nx commented Sep 29, 2020

Interesting! Do you have an example of a 2theta arm rotation and what it looks like?

@benjaminhwilliams
Copy link

Sorry for the slow reply regarding the non-zero detector 2θ. Here is an example from the chemical crystallography beamline I19 at Diamond Light Source. Unfortunately it is only a single-panel detector but everything seems to behave OK. Testing on a single CBF image from this archive, which has a detector 2θ position of 30°, we see
$ dxtbx.plot_detector_models orthographic=True l-cyst_01_00001.cbf
image

$ dxtbx.plot_detector_models orthographic=True project_onto=image_plane l-cyst_01_00001.cbf
image

The difference isn't too apparent with this small 2θ, so if we doctor the image file to have 2θ = 89°, we can see the difference more clearly.
$ dxtbx.plot_detector_models orthographic=True l-cyst_01_00001.cbf
image

$ dxtbx.plot_detector_models orthographic=True project_onto=image_plane l-cyst_01_00001.cbf
image

If we use 2θ = 90°, the edge-on projection onto the lab x-y plane causes no problems:
$ dxtbx.plot_detector_models orthographic=True l-cyst_01_00001.cbf
image

$ dxtbx.plot_detector_models orthographic=True project_onto=image_plane l-cyst_01_00001.cbf
image

Finally, if we go beyond the vertical and set 2θ = 120°, everything still looks sensible:
$ dxtbx.plot_detector_models orthographic=True l-cyst_01_00001.cbf
image

$ dxtbx.plot_detector_models orthographic=True project_onto=image_plane l-cyst_01_00001.cbf
image

Though in this last case, we cannot determine which way up the image-plane projection is.

Copy link
Contributor

@phyy-nx phyy-nx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

@dagewa
Copy link
Member Author

dagewa commented Oct 6, 2020

Thanks @benjaminhwilliams and @phyy-nx. As this doesn't change behaviour outside of adding an option to dxtbx.plot_detector_models I think it is safe to merge, and I plan to do that today.

@Anthchirp
Copy link
Member

The test failures are not a fluke: This adds a dxtbx > scikit-learn dependency.

@dagewa
Copy link
Member Author

dagewa commented Oct 6, 2020

Ah, gotcha, I forgot to look at the Azure tests. There's a single-panel mode that doesn't use the clustering. This could be used for multi-panel detectors where sklearn is not available, but the end result of the projection may be different. Adding a dependency requires wider discussion, so then I won't merge yet.

Without sklearn, multiple panel detectors will be treated like single
panel detectors.

if clustered_axes:
summed_axes = [matrix.col((0, 0, 0)) for i in range(nclusters)]
for axis, cluster in zip(axes, clusters):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definition of cluster here shadows the imported name, leading to the unexpected test failure

@codecov
Copy link

codecov bot commented Oct 7, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@f227287). Click here to learn what that means.
The diff coverage is 66.18%.

@@            Coverage Diff            @@
##             master     #224   +/-   ##
=========================================
  Coverage          ?   45.31%           
=========================================
  Files             ?      228           
  Lines             ?    19412           
  Branches          ?     2754           
=========================================
  Hits              ?     8797           
  Misses            ?    10097           
  Partials          ?      518           

model/detector_helpers.py Outdated Show resolved Hide resolved
model/detector_helpers.py Outdated Show resolved Hide resolved
dagewa and others added 2 commits October 7, 2020 22:13
@dagewa
Copy link
Member Author

dagewa commented Oct 16, 2020

As all checks are now passing, I'll merge this.

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.

4 participants