Resolve validation issues caused by imgCIF

[vaitkus]

Since the addition of the imgCIF import statement some of our automated tests no longer pass. What is worse, some of the more formal tests (formal validation against the ddlm reference dictionary) are not run at all.

There are several ways we could try to resolve this:

1. Properly clean up the DDLm version of the imgCIF dictionary.

2. Skip the cod-tools based test altogether for now.

3. Update the checker with ad-hoc rules to ignore issues relating to the missing imgCIF dictionary import and the resulting side-effects (e.g. unrecognised categories).

4. will probably take the most time, 2) somewhat defeats the purpose while 3) would require some minor hacking on my part, but seems manageable.

[jamesrhester]

I'm currently working on (1). My ongoing attempts to get as few remaining issues as possible from the cod-tools checker can be seen here.

What I think we might end up doing is creating two DDLm versions: one which is suitable for lossless conversion back to DDL2 (so retain the ddl2-specific attributes and text formatting) and one which is semantically equivalent from a DDLm perspective and suitable for importing and automated checking. Some examples of round-trip issues:

1. COD checks wants version numbers presented in ascending order, whereas DDL2 has the opposite.
2. Line length and many text-valued items violate style rules. Reformatting would cause difficulty comparing when round-tripping.
3. The ddl2 version includes definitions for data names aliased in other definitions, which is flagged by the COD checker, but removing them would change what is output when converting back to ddl2.
4. The dictionary title must be changed to match DDLm conventions.

The current process involves automated transformation using dREL embedded in definitions, followed by post-processing that roughly speaking doesn't change any of the semantics. It's all contained in ddl_to_ddl which hasn't yet been updated with my recent changes.

[jamesrhester]

The current status of automated translation is here. There are only two types of error remaining:

1. Data names aliased via imgCIF and via cif_core end up in different headline definitions, which COD tools doesn't like. These are all datanames of the form _diffrn_*, as these are common across mmCIF/imgCIF and cif_core. I will fix this in the generated DDLm to conform to the cif_core perspective on which data name is the canonical one.
2. COD tools warns about _array_intensities.gain_esd not ending in _su. We should set COD-tools checking to ignore this warning, as we can't change the data name.

[vaitkus]

Great thank you for the work.

1. The main problem here is that we end up with the same name in two differently named save frame, therefore the import operation cannot automatically overwrite coreCIF definitions with equivalent imgCIF operations. The suggested post processing seems reasonable.
2. Is _array_intensities.gain_esd a true esd or is it an SU? If so, maybe we could also post process this definition by:
3. Renaming the save frame to _array_intensities.gain_esd.
4. Add _array_intensities.gain_su as the main data name.
5. Add _array_intensities.gain_esd as an alias.
   Or is that too much creative liberty?

[jamesrhester]

I don't think we should add data names simply to fulfill a stylistic desire. In any case, the _esd ending is the mmCIF equivalent of our _su. _array_intensities.gain_esd is also defined in mmCIF which suggests that they consider esd and su to be the same.

[jamesrhester]

I have finished updating cif_img.dic: see COMCIFS/imgCIF#17

Only the _esd issue should remain, which I think we should ignore in the check if possible (option (3) above).

[jamesrhester]

See #268.

[vaitkus]

Thanks, I will add the exceptions to the cod-tools based checker, hopefully this week.

[vaitkus]

However, I think only the issues raised by the cif_ddlm_dic_check script were addressed. There are still the formal validation issues that are detected by the ddlm_validate script:

ddlm_validate -d ddl.dic cif_img.dic -I ../MultiBlock_Dictionary/ -I ../cif_core/ --follow-iucr-style-guide

After I add the _array_intensities.gain_esd exception, these issues should also become visible in the workflow (currently it fails early and simply does not reach that validation step).

[vaitkus]

Merging PR COMCIFS/dictionary_check_action#17 should suppress the esd data name issue. Additional formal validation messages generated by the ddlm_validate script now appear as expected.

[jamesrhester]

Those don't look to difficult, I'll update here once I've addressed them.

[jamesrhester]

It looks like all issues are solved. See COMCIFS/imgCIF#18.