Skip to content

Updates from NIAC 2016#568

Merged
zjttoefs merged 2 commits intomasterfrom
nxreflections_niac2016
Jun 8, 2017
Merged

Updates from NIAC 2016#568
zjttoefs merged 2 commits intomasterfrom
nxreflections_niac2016

Conversation

@phyy-nx
Copy link
Copy Markdown
Contributor

@phyy-nx phyy-nx commented Jun 5, 2017

Action items from minutes completed:
-H,K,L change to NX_number for incommensurate structures
-Discussion on flags, change to bitfield, no enum for efficiency
-Expand the description of partiality
-Need to keep mm and pixel positions because of parallax
-Write out predicted and observed
-Drop the mm in names, rather use units
-bounding_box as array, document the usage
-Spell out background
-Drop the val on the intensities, keep the _var version
code is not in place we defer it for now.
-Add polar_angle and azimuthal_angle

TODO
-Need documentation for polar_angle and azimuthal_angle
-Need to add some examples of flags. Would like to use the DIALS flags.

Deferred:
-overlaps is a list of overlapped reflections. This is a ragged array. As the

Action items from minutes completed:
-H,K,L change to NX_number for incommensurate structures
-Discussion on flags, change to bitfield, no enum for efficiency
-Expand the description of partiality
-Need to keep mm and pixel positions because of parallax
-Write out predicted and observed
-Drop the mm in names, rather use units
-bounding_box as array, document the usage
-Spell out background
-Drop the val on the intensities, keep the _var version
code is not in place we defer it for now.
-Add polar_angle and azimuthal_angle

TODO
-Need documentation for polar_angle and azimuthal_angle
-Need to add some examples of flags. Would like to use the DIALS flags.

Deferred:
-overlaps is a list of overlapped reflections. This is a ragged array. As the
Copy link
Copy Markdown
Contributor

@zjttoefs zjttoefs left a comment

Choose a reason for hiding this comment

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

The bbx could have the correct dimensions enforced.

Otherwise this looks like a nice improvement (without checking the NIAC minutes). I'll defer merging so you can add the commits to the remaining TODOs here, though. If you prefer an early merge, let me know.

Add DIALS labels to the bits in the flags bitmask
Rename bbx to bounding_box
Add dimensions to bounding_box
Add docstrings to polar and azimuthal angles
Copy link
Copy Markdown
Contributor

@zjttoefs zjttoefs left a comment

Choose a reason for hiding this comment

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

I like the new "bounding_box" name and the added documentation for the flags and angles.

Since the first dim "n" for the bounding box is never used elsewhere (as far as I see), that might be dropped. But it might come in handy as well.

@zjttoefs zjttoefs merged commit aeb9816 into master Jun 8, 2017
@zjttoefs
Copy link
Copy Markdown
Contributor

zjttoefs commented Jun 8, 2017

This should have addressed all NIAC comments.

@prjemian
Copy link
Copy Markdown
Contributor

prjemian commented Jun 9, 2017

In the flags fields, there is bit 17: bad_shoebox. It is not obvious what is a shoebox in this context.

@phyy-nx phyy-nx deleted the nxreflections_niac2016 branch June 13, 2017 15:54
@phyy-nx
Copy link
Copy Markdown
Contributor Author

phyy-nx commented Jun 13, 2017

dials/dials#363 aims to document all these flags, including bad_shoebox. When those doc strings are in place, I'll put them here.

Question about the first dim "n" for the bounding box. Should all the other columns have 'n' as a dimension? For example, 'h', 'k', and 'l' should all be arrays of dimension n.

@prjemian
Copy link
Copy Markdown
Contributor

Should all the other columns have 'n' as a dimension? For example, 'h', 'k', and 'l' should all be arrays of dimension n.

The existing NXDL files are not consistent in this regard. Yours is a good idea, fits the current circumstances, and would inspire us to revise NXDL files similarly. One reason for the inconsistency in other NXDL files is that some fields have a variable shape.

@phyy-nx
Copy link
Copy Markdown
Contributor Author

phyy-nx commented Jun 13, 2017 via email

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