COMCIFS / cif_core

The IUCr CIF core dictionary
15 stars 9 forks source link

Enumeration range for cell lengths #495

Closed nautolycus closed 2 months ago

nautolycus commented 3 months ago

Apologies if this has been considered before. The DDL1-based dictionary definition of the _cell_length_a etc. items has

_enumeration_range 0.0:

but the current core has e.g.

_enumeration.range 1.:

The enumeration range is imported from save_cell_length in templ_attr.cif and this seems to have entered at commit https://github.com/COMCIFS/cif_core/commit/ada87185fd1d88ce70cb5c7f3c15b1609164d893

I tend to favour the view that the enumeration ranges in the dictionary should be confined to values that are physically possible rather than what might be thought "reasonable" (leaving the latter to validation applications or overlays), so prefer the "non-negative" constraint rather than an arbitrary 1-angstrom cutoff. Any thoughts?

vaitkus commented 3 months ago

@nautolycus this was indeed previously somewhat discussed in issue #64. The conclusion for the _cell_length_* items was to choose 1.0: as the new enumeration range based on the radius of a hydrogen atom and to backport this change to the legacy version of the DDL1 CIF_CORE dictionary (see https://github.com/COMCIFS/DDL1-legacy-dictionaries/blob/main/dictionaries/cif_core.dic where is change is already present since around 2017).

However, I think that the enumeration range can be changed (back) to 0.0: in both the DDL1 and the DDLm versions if needed without having a significant effect on the validation of existing CIF files. Personally, I also like the 0.0: enumeration range better, but maybe @jamesrhester knowns the original reasoning that led to this change when migrating from DDL1 to DDLm?

jamesrhester commented 3 months ago

I have no insight to offer here, beyond supposing that perhaps certain dREL methods may have failed if provided with a zero-valued cell length, and so a minimal sensible value was chosen. I'm pretty certain there was no public discussion of this change, so reverting it would not be a problem. I have no objection to going back to 0.0.