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

Restore _space_group.crystal_system validation method #262

Open
merkys opened this issue Oct 8, 2021 · 3 comments
Open

Restore _space_group.crystal_system validation method #262

merkys opened this issue Oct 8, 2021 · 3 comments

Comments

@merkys
Copy link
Contributor

merkys commented Oct 8, 2021

Currently cif_core.dic does not contain any validation methods expressed in dREL, although it had prior to 8ca3b54. Said commit had removed validation method for _space_group.crystal_system as it contained undefined dREL functions throw and alert. I think it could be restored and transformed into a statement evaluating to a boolean value, as validation methods should per Spadaccini et al., 2012.

@jamesrhester
Copy link
Contributor

My thinking on "validation" methods was that they can simply be "evaluation" methods for a separate data name that would be part of a validation dictionary, eventually replacing CheckCIF. One reason for this is that there may be multiple ways of checking a single value, and by assigning separate data names to each evaluation fine-grained error messages may be returned. If we put validation methods into CIF core, an automated checker will only be able to say that there is a problem with some value of some data name, but be unable to provide further information.

One advantage of separate validation data names is that non-boolean results can be provided, e.g. if the validation data name has an enumerated set as a value, with _enumerated_set.detail explanations, those explanations can be used to automatically explain the nature of the problem.

Another advantage is that a separate "checking" dictionary would cleanly separate the meaning of a data name from the checking.

It would be a great service to the community to put together such a "validation data names" dictionary. I have started this project a few times and had to abandon it due to more pressing issues, but if anyone else wants to start a collection they are more than welcome. There is also scope for auto-generating a whole batch of checking definitions using the existing 'Evaluation' methods, along the lines of "these values are inconsistent".

@merkys
Copy link
Contributor Author

merkys commented Oct 20, 2021

Thanks for explanation. I was unaware of this idea and this seems to be a clean way to separate definitions from validation methods. My initial thought about retaining the original idea from Spadaccini et al., 2012 at the same time having fine-grained validation messages was to split checks:

save_space_group.it_number
loop_
_method.purpose
_method.expression
_method.explanation # NB: not in dictionaries
Validation  'number_is_natural(_space_group.it_number)'  'space group number must be natural number'
Validation '_space_group.it_number <= 230'               'space group number must not be greater than 230'

(I am aware there are simpler methods to enforce these constraints, just adding this for example)

This way every failed check could be reported instead of just saying that there is an issue with the value.

@jamesrhester
Copy link
Contributor

Yes, I feel that embedding a potentially large number of potentially quite long checks in the definition will lead to clutter, as well as there not being a logical home for checks that aren't local to a particular value (e.g. a group of values being inconsistent).

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