Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
dials.export: Add option to specify composition of asymmetric unit so a complete SHELX instruction file can be written. #2623
dials.export: Add option to specify composition of asymmetric unit so a complete SHELX instruction file can be written. #2623
Changes from 6 commits
81f7ffc
8c3999f
65c28f7
c450079
a7db41b
322b31b
fcb7fe9
b540afe
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A thought occurs to me. Does SHELX input support non-stoichiometric formulae? Should
int
befloat
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems it is not an error for
shelxl
, but it would be unconventional. I haven't found any examples of non-integer values afterUNIT
online. Wouldn't this just be represented by the occupancy column for the atom?As for
shelxt
, it ignores what's inUNIT
entirely.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UNIT
is element count in unit cell not asu. I was multiplying the formula entered by Z. Quite common to have fractions of the formula in the asu but fractions of an atom in the unit cell ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep this as int. Convenient to have the composition multiplied by Z 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I now understand @benjaminhwilliams's comment. Take an example like CCDC 1880252. In that case I think with the current code you'd want to write 'C6 H5 B0.25 F P0.25' if you scaled the data in I-4. So maybe we should have
float
here and convert to int when writing theUNIT
card? Or allow users to set Z (as in_cell_formula_units_Z
) to overridespace_group().order_z()
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, gotcha. Allow float input and convert to int makes sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking some more about this probably the best approach is to not allow non-integer values but add a parameter
shelx.zprime
so for the example above the user would enter 'C24 H20 B F4 P' andzprime=0.25
and get Z = 2.In that case should
shelx.composition
be renamedshelx.formula
? or should we have both - whereshelx.composition
is just element types andshelx.formula
is always interpreted as a formula even when there no numbers.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth adding a test which actually executes
shelxt
if available to verify?