adding beta ADPS

[rowlesmr]

Will close #354

My initial go. More to come.

[vaitkus]

As discussed in #354, we should add an appropriate enumeration value to the _atom_site.ADP_type data item. Value \bani seems a bit too close to the already existing Bani so maybe beta_ani?

[rowlesmr]

maybe beta_ani?

I added betaani; no underscore just to maintain consistency with the other options, but not desperate to keep it out if change is wanted.

[rowlesmr]

aniso_BIJ2 in templ_attr.cif isn't imported anywhere in cif_core.dic. Does anyone know it's purpose? It appears in ada8718.

[jamesrhester]

aniso_BIJ2 in templ_attr.cif isn't imported anywhere in cif_core.dic. Does anyone know it's purpose?

I suspect that at some point these were going to be used to define the Bij components, and for whatever reason that was abandoned. Perhaps they were included in a draft at some point (20 years ago?) as an intermediate calculation step.

[jamesrhester]

How does this relate to _atom_site.tensor_beta? Aren't these just the components?

[rowlesmr]

Should _atom_site.tensor_beta be deprecated, or is having it as an alias enough?

Edit: Alias leads to double-definition. Deprecation, it is.

[vaitkus]

aniso_BIJ2 in templ_attr.cif isn't imported anywhere in cif_core.dic. Does anyone know it's purpose?

I suspect that at some point these were going to be used to define the Bij components, and for whatever reason that was abandoned. Perhaps they were included in a draft at some point (20 years ago?) as an intermediate calculation step.

@jamesrhester should/could we remove these save frames from templ_attr.cif?

[rowlesmr]

Also note that in the new dREL (which is a duplication of the old dREL), not all _atom_site.adp_type enumeration values are allowed for; specifically, if _atom_site.adp_type has the value Uovl, Umpe, or Bovl, it will attempt to do U = b.B_iso_or_equiv / (8 * Pi**2). Should the logic specifically disallow this execution path?

[vaitkus]

Also note that in the new dREL (which is a duplication of the old dREL), not all _atom_site.adp_type enumeration values are allowed for; specifically, if _atom_site.adp_type has the value Uovl, Umpe, or Bovl, it will attempt to do U = b.B_iso_or_equiv / (8 * Pi**2). Should the logic specifically disallow this execution path?

I think that for now it is sufficient to explicitly specify that the U = b.B_iso_or_equiv / (8 * Pi**2) should only be executed if _atom_site.adp_type == 'Biso'. We still end up with an undefined U in UIJ = U * _cell.convert_Uiso_to_Uij, but I guess this is OK in dREL and will simply produce an undefined value (@jamesrhester please correct if I am wrong here). The appropriate code for Uovl, Umpe and Bovl can be added later down the line if such conversions are even possible.

[rowlesmr]

How are Uovl, Umpe and Bovl actually used?

Does each atom_site get an identical U|B value, and also marked as Uovl|Bovl? If so, then we can just treat them as Uiso|Biso.

I can't even Umpe.

[vaitkus]

Two remaining things that need to be decided on before merging the PR:

• Should we remove the aniso_bij2 save frame from the templ_attr.cif since it does not seem to be used anywhere and is probably more or less fully replace by aniso_bij.
• Should the dREL under _atom_site_aniso.matrix_beta be updated to handle Uovl, Umpe and Bovl ADP values? If yes, then how such values should be handled?

As for the usage of Uovl and Bovl ADP types in general, the COD only contains a handful of such entries (~16 Uovl, ~3 Bovl). In those files, sometimes all of the atoms contain the Uovl type and sometimes only some. Similarly, sometimes all of the Uovl atoms contain the same ADP value and sometimes the values differ. If the IUCr had a specific usage in mind, this is not well reflected in the actual files.

No entries in the COD contained the Umpe type.

Edit: added some information on the usage of Uovl and Bovl.

[rowlesmr]

• Should we remove the aniso_bij2 save frame from the templ_attr.cif since it does not seem to be used anywhere and is probably more or less fully replace by aniso_bij.

I think it should be OK to remove it.

• Should the dREL under _atom_site_aniso.matrix_beta be updated to handle Uovl, Umpe and Bovl ADP values? If yes, then how such values should be handled?

I need to actually know what they are before I can implement anything (I've laid hands on a copy of Willis and Pryor, are they in there?), but also given that there isn't really any use of them, we could safely ignore them.

[rowlesmr]

I think we can safely ignore Umpe; reading Multipole density formalism and slide 23 leads me to believe CIF can't even record the values required to have values for Umpe, let alone convert them to beta values...

Following that, we could also ignore the ovl specialisations if there's only a dozen or so of them.

[jamesrhester]

I agree that aniso_bij2 can be removed. I don't think we need to change the matrix_beta dREL at this stage (it remains correct). In the future, when we can have multiple dREL methods available, then we might consider providing alternative routes to calculating quantities in CIF.

In which case, we are ready to merge.