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.

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

Add AUDIT_CONFORM category loops to the revelant files under examples/ #11

Open
vaitkus opened this issue Mar 18, 2024 · 8 comments
Open

Comments

@vaitkus
Copy link
Collaborator

vaitkus commented Mar 18, 2024

CIF users are generally encouraged to explicitly specify the CIF dictionary that their CIF files conform two. The same approach should be used in the example CIF files provided in this repository.

The AUDIT_CONFORM loop could look something like this, although the dictionary location could be changed to point to a more stable resource :

loop_
_audit_conform.dict_name
_audit_conform.dict_version
_audit_conform.dict_location
MULTIBLOCK_DIC 1.1.0
https://raw.githubusercontent.com/COMCIFS/MultiBlock_Dictionary/main/multi_block_core.dic

Also, it is not yet completely clear if the AUDIT_CONFORM loop should be placed in all data blocks in a file or if it is sufficient to only place it in one of the files.

This suggestion was originally raised in a discussion thread of PR #6 (see https://github.com/COMCIFS/MultiBlock_Dictionary/pull/6/files#r1479649381).

@jamesrhester
Copy link
Contributor

This does require a decision on how the AUDIT_CONFORM category behaves when blocks are combined. The choices I see are:

  1. We invent an identifier for an AUDIT_CONFORM domain to which every single data name can be associated. This would allow mixing data name conformance from different versions of the same dictionary at the cost of a new data name for every category
  2. We make AUDIT_CONFORM special, e.g. it applies to the data block it appears in and vanishes when blocks are merged
  3. We do nothing, in which case AUDIT_CONFORM appearing in one data block applies to all data blocks.

I strongly favour (3). Users are welcome (strongly encouraged, even) to put an AUDIT_CONFORM loop in every data block, but the contents must not contradict the AUDIT_CONFORM loops in other data blocks. That means the same version for each named dictionary, as _audit_conform.dict_name is the key data name. This seems like a reasonable base constraint.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Apr 3, 2024

I am also in favour of option (3). Handling and merging data blocks that conform to different versions of the same dictionaries does not seem feasible in practice. Should this additional constraint be formally written down somewhere, e.g. in the multiblock dictionary?

@jamesrhester
Copy link
Contributor

Option (3) is the "no special behaviour" option, so I think the best place to describe how AUDIT_CONFORM works is in the examples and material in Volume G covering this dictionary, which would also advise putting it into all data blocks as good practice.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Apr 4, 2024

Understood. By "additional constraint" I mainly meant the requirement that the contents of the AUDIT_CONFORM loops are compatible between different blocks. This definitely means that there cannot be several dictionaries with the same name and differing parameters (e.g. different version numbers), but what about dictionaries, that override some data names? For example, could we have two blocks, one in conformance to CIF_CORE and one in conformance to PD_CIF, both of which contain the looped _refln.f_squared_meas data item (_refln.f_squared_meas is originally defined in CIF_CORE and then redefined in PD_CIF)?

@jamesrhester
Copy link
Contributor

I intended only compatibility in the relational sense, as for any other loop.

Your question is, however, a fine one and I think opens up a whole can of worms that deserves its own discussion.

@jamesrhester
Copy link
Contributor

jamesrhester commented Jul 17, 2024

As outlined in #13 , the answer I propose is:

  1. If all data blocks conform to the same list of mutually compatible dictionaries, only one block needs to contain the audit_conform loop.
  2. If at least one data block does not conform to the same list of mutually compatible dictionaries, then all data blocks should contain an audit_conform loop
  3. Dictionaries are mutually compatible if they define the same key data names for all categories and assume the same calculation methods for all data names that they have in common.
  4. Good practice suggests putting an audit_conform loop in all data blocks, in case a data block is later appended that contains a different set of dictionaries (e.g. powder data combined with single crystal data later).

@vaitkus
Copy link
Collaborator Author

vaitkus commented Jul 17, 2024

Looks great overall, however, I would probably change "should contain" to "must contain" in point 2. Also, I really like the recommendation in point 4!

@jamesrhester
Copy link
Contributor

Indeed. "Must contain" is correct.

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

No branches or pull requests

2 participants