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.

Sign up for GitHub

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

Jump to bottom

Add probe type to the Beam model #647

Merged
merged 23 commits into from
Aug 4, 2023
Merged

Add probe type to the Beam model #647

merged 23 commits into from
Aug 4, 2023

Conversation

dagewa
Copy link
Member

@dagewa dagewa commented Jul 24, 2023

Fixes #575.

This PR adds a Probe enum to Beam. In future this could be used to alter behaviour of algorithms, such as the QE and parallax corrections. At the moment this just adds the additional datum and allows serialisation etc. Old experiment files where this is not set are interpreted as having Probe.xray. Some electron diffraction format classes are updated here to record Probe.electron.

A DIALS PR will be required following this. Currently the only DIALS tests that fail are those for dials.show.

dagewa added 13 commits July 21, 2023 15:36
@dagewa
Add a probe enum
c981f71
@dagewa
get_probe accessor
bae2822
@dagewa
probe-->Probe, and export to Python.
6eea2fd 
So now, can do

from dxtbx.model.beam import Probe
@dagewa
Export get_probe
eb22c37 
Note the value is uninitialized!
@dagewa
Ensure probe_ is initialized. Now we get:
2b79fd8 
>>> from dxtbx.model.beam import Beam
>>> b=Beam()
>>> b.get_probe()
dxtbx_model_ext.Probe.xray
@dagewa
Add set_probe and export.
6979e25 
Now this works:

>>> from dxtbx.model.beam import Beam
>>> b=Beam()
>>> b.get_probe()
dxtbx_model_ext.Probe.xray
>>> from dxtbx.model.beam import Probe
>>> b.set_probe(Probe.neutron)
>>> b.get_probe()
dxtbx_model_ext.Probe.neutron
@dagewa
Ensure probe is initialised by the other constructors too
e5c8839
@dagewa
Incorporate probe into equality and similarity checks
48c096d
@dagewa
Add get_probe_name method so that the NeXus probe name can be
899b166 
returned for each of the defined probe values
@dagewa
Include the probe in to/from dict methods and pickle suite.
463391b
@dagewa
Add probe to make_polarized_beam, and fix longstanding bug in that me…
0b08c5d 
…thod
@dagewa
Set electron probe for various 3D ED format classes
f5e5441
@dagewa
News
9f2400a
@dagewa dagewa linked an issue Jul 24, 2023 that may be closed by this pull request
Beam model should state probe type #575
Closed
@codecov
Copy link

codecov bot commented Jul 24, 2023 •
edited
Loading

Codecov Report

Merging #647 (fb30aa9) into main (1625cfa) will decrease coverage by 0.30%.
Report is 9 commits behind head on main.
The diff coverage is 29.12%.

❗ Current head fb30aa9 differs from pull request most recent head 827e494. Consider uploading reports for the commit 827e494 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #647      +/-   ##
==========================================
- Coverage   39.39%   39.09%   -0.30%     
==========================================
  Files         178      179       +1     
  Lines       15519    15819     +300     
  Branches     2996     3057      +61     
==========================================
+ Hits         6113     6185      +72     
- Misses       8827     9052     +225     
- Partials      579      582       +3

toastisme
toastisme approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@toastisme toastisme left a comment

Choose a reason for hiding this comment

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

This all looks fine to me, and should be quite useful.

dagewa added a commit to dials/dials that referenced this pull request Aug 2, 2023
@dagewa
Update dials.show tests for cctbx/dxtbx#647
daa91a7
@dagewa dagewa mentioned this pull request Aug 2, 2023
Merged
@dagewa dagewa requested a review from toastisme August 2, 2023 16:44
@dagewa
Copy link
Member Author

dagewa commented Aug 2, 2023

@toastisme I've made the changes to hopefully get Probe working with the PolychromaticBeam. Might have missed something, so if you have anything else you can check on your fork that would be good. One thing I wasn't sure of were the type hints for a value like Probe.xray, so I just left that blank.

@toastisme
Added get_probe_from_name to Beam. Added Probe to beam_phil_scope. Ch…
1f3c257 
…anged Probe in .expt files to be human readable.
@toastisme
Added more typehints for beam.py. Added empty PolychromaticBeam const…
4312daf 
…ructor at the Python level. Fixed typo in Beam.from_dict.
@toastisme
Copy link
Contributor

@toastisme I've made the changes to hopefully get Probe working with the PolychromaticBeam. Might have missed something, so if you have anything else you can check on your fork that would be good. One thing I wasn't sure of were the type hints for a value like Probe.xray, so I just left that blank.

Had a look and looks good. I realised Probe wasn't part of the phil params so have added it here, along with a suggestion to have the probe be the NeXus name in .expt files, rather than the enum int, to make it human readable.

@dagewa
Merge pull request cctbx#654 from toastisme/beam-probe-types
35a312c 
Add probe to phil params and make probe in .expt human readable
@dagewa dagewa mentioned this pull request Aug 3, 2023
Merged
@dagewa
Merge pull request #187 from cctbx/575-beam-model-should-state-probe-…
5bdb3b4 
…type

575 beam model should state probe type
ndevenish
ndevenish requested changes Aug 4, 2023
View reviewed changes
src/dxtbx/model/beam.h Outdated Show resolved Hide resolved
src/dxtbx/model/beam.h Outdated Show resolved Hide resolved
dagewa and others added 2 commits August 4, 2023 14:27
@dagewa @ndevenish
Update src/dxtbx/model/beam.h
f467e48 
Co-authored-by: Nicholas Devenish <[email protected]>
@dagewa @ndevenish
Update src/dxtbx/model/beam.h
827e494 
Co-authored-by: Nicholas Devenish <[email protected]>
@ndevenish ndevenish merged commit 4783b2c into cctbx:main Aug 4, 2023
11 checks passed
@ndevenish ndevenish deleted the 575-beam-model-should-state-probe-type branch August 4, 2023 15:48
@ndevenish ndevenish mentioned this pull request Aug 15, 2023
Merged
@dagewa dagewa mentioned this pull request Oct 11, 2023
Closed
toastisme added a commit to toastisme/dxtbx that referenced this pull request Jul 18, 2024
@dagewa @toastisme
Add probe type to the Beam model (cctbx#647)
18ac682 
At the moment this just adds the additional datum and allows
serialisation etc. Old experiment files where this is not set are
interpreted as having Probe.xray. Some electron diffraction format
classes are updated here to record Probe.electron.

Co-authored-by: davidmcdonagh <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ndevenish ndevenish ndevenish approved these changes

@graeme-winter graeme-winter Awaiting requested review from graeme-winter

@toastisme toastisme Awaiting requested review from toastisme

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Beam model should state probe type
4 participants
@dagewa @toastisme @ndevenish @DiamondLightSource-build-server