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

cif_core: newly granted s.u. eligibility due to the measurand purpose #107

Open
vaitkus opened this issue Dec 31, 2018 · 10 comments
Open

cif_core: newly granted s.u. eligibility due to the measurand purpose #107

vaitkus opened this issue Dec 31, 2018 · 10 comments

Comments

@vaitkus
Copy link
Collaborator

vaitkus commented Dec 31, 2018

Some data items were marked as 'Measurand' in the process of converting the core dictionary from DDL1 to DDLm and as such became eligible to be accompanied by a standard uncertainty value (s.u.). However, not all of the affected data items are allowed to have s.u. values in the DDL1 dictionary. Some of the changes seem totally reasonable (i.e. items from the EXPTL_CRYSTAL_SIZE category) while other seem a bit strange (i.e. items from the ATOM_SITES_CARTN_TRANSFORM category).

It would great if somebody could check if all of these changes were intentional before I try to port them to the DDL1 version of the dictionary.

The following item are eligible for s.u. values in the DDLm version, but not in the DDL1 version:

  • _diffrn_orient_refln_angle_[chi, kappa, omega, phi, psi, theta];
  • _diffrn_refln_counts_[bg_1, bg_2, net, peak, total];
  • _diffrn_refln_intensity_net;
  • _diffrn_scale_group_i_net;
  • _diffrn_standards_scale_[sigma, u];
  • _refln_[a, b, f, f_squared]_[calc, meas];
  • _refln_intensity_[calc, meas];
  • _refln_phase_calc;
  • _cell_measurement_refln_theta;
  • _chemical_melting_point_[lt, gt];
  • _chemical_temperature_decomposition_[lt, gt];
  • _chemical_temperature_sublimation_[lt, gt];
  • _exptl_crystal_density_diffrn;
  • _exptl_crystal_density_meas_[lt, gt];
  • _exptl_crystal_density_meas_temp_[lt, gt];
  • _exptl_crystal_size_[length, min, mid, max, rad];
  • _exptl_crystal_face_diffr_[chi, kappa, phi, psi];
  • _exptl_crystal_face_perp_dist;
  • _atom_type_analytical_mass_%;
  • _chemical_formula_weight_meas;
  • _geom_bond_valence;
  • _atom_sites_cartn_tran_matrix_[1, 2, 3][1, 2, 3];
  • _atom_sites_cartn_tran_vector_[1, 2, 3];
  • _atom_sites_fract_tran_matrix_[1, 2, 3][1, 2, 3];
  • _atom_sites_fract_tran_vector_[1, 2, 3];
  • _refine_ls_restrained_s_[all, obs, gt].
@jamesrhester
Copy link
Contributor

As far as I can tell almost all of these quantities could have an SU attached, as their derivation or measurement could involve quantities that have SU. The only quantities that seem unusual are the _refln_[a,b,f,f_squared]_calc values, which are calculations from the model. So I think it will be necessary to change everything except those in DDL1.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Mar 26, 2019

I just want to bring up a few additional points before proceeding with changes to the DDL1:

The _diffrn_standards_scale_[sigma, u] items

This data item holds the average standard uncertainty value and as such should probably not
be eligible for an su values itself. Formal definition from the dictionary:
The average standard uncertainty of the individual standard scales applied to the intensity data.

Less than/greater than items

At one point in time, the lt/gt items in the DDL1 based dictionaries were also allowed to contain standard
uncertainties. However, later on they were removed following a suggestion by Gotzon Madariaga: since these are ceiling/floor values a measurable uncertainty is pointless (CIF_CORE dictionary, _dictionary_history, entry 2003-09-29). Maybe the same reasoning should be followed in DDLm as well?

However, I do agree that it is easy to come up with an example where one would want to record the lower or upper bound with a standard uncertainty.

Items that fall under this case:

  • _chemical_melting_point_[lt, gt];
  • _chemical_temperature_decomposition_[lt, gt];
  • _chemical_temperature_sublimation_[lt, gt];
  • _exptl_crystal_density_meas_[lt, gt];
  • _exptl_crystal_density_meas_temp_[lt, gt];

Please let me know if these points change anything. If not, I will update the affected DDL1 definitions with the exclusion of the following data items:

  • _refln_a_calc;
  • _refln_b_calc;
  • _refln_f_calc;
  • _refln_f_squared_calc.

@jamesrhester
Copy link
Contributor

Regarding _diffrn_standards_scale_[sigma,u] isn't it true that an average can have an uncertainty? So while it seems rather pointless to include it, it is possible. The lt/gt values comment is true though, and we shouldn't be second-guessing decisions already documented without a good reason. I think in the lt/gt case as long as the stated bound is at least 3su from any measured value in the "safe" direction, and then an SU is not necessary.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Sep 23, 2021

PR #254 introduced separate SU items to all Measurand items, including the lt/gt ones. How does this affect the status of this issue?

Looking at the previous comment of @jamesrhester , it seems that it was agreed to not allow lt/gt items to have SU values. Did this change or should the related SU items be removed and the definitions of the lt/gt items modified in some way (i.e. by changing the purpose from Measurand to Number)? On the one hand, the lower/upper bounds are still measured in some way and thus can contain SU values. On the other hand, previous dictionary maintainers made a calculated decision to remove these SU values (see the comment of Gotzon Madariaga that I cited in my previous comment #107 (comment)).

@jamesrhester
Copy link
Contributor

jamesrhester commented Sep 24, 2021

No, there has not been any ensuing discussion. I think the _gt/_lt items should be changed to Number and SU removed. _gt/_lt items should include in their definitions that the true value should lie within the stated bound with a likelihood > 99.9%. Where necessary 3su of the measurement error should be applied to any measured value to satisfy this requirement.
I will do this in a separate PR, but first I'll see if Gotzon on Brian remember the discussion, as the list archives don't seem to include this comment.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Nov 10, 2021

PR #281 resolved the issue with _gt/_lt data items by reverting their purpose back to Number and removing the associated SU items.

One last thing. In a previous comment by @jamesrhester, it was agreed that SU values of the _refln_[a,b,f,f_squared]_calc data items also look quite strange and should not be backported to DDL1:

The only quantities that seem unusual are the _refln_[a,b,f,f_squared]_calc values,
which are calculations from the model. So I think it will be necessary to change everything
except those in DDL1.

Maybe the the purpose of these items should also be reverted back to Number in DDLm as well? Data items in question:

    _refln_a_calc
    _refln_b_calc
    _refln_f_calc
    _refln_f_squared_calc

@jamesrhester
Copy link
Contributor

I may have been hasty in that these calculated values can have an SU associated with them if they are calculated from parameters that have an SU associated with them. Of course systematic errors in e.g. form factors are not captured but that's no different to systematic errors in measurements. So perhaps it is best to leave them as they are.

@vaitkus
Copy link
Collaborator Author

vaitkus commented Nov 12, 2021

Ok, thank you for the clarification. I think that this issue can now be considered resolved.

However, I have opened a separate issue in the legacy dictionary repository that deals with actually implementing these changes in the DDL1 version of the dictionary (see COMCIFS/DDL1-legacy-dictionaries#5).

@vaitkus vaitkus closed this as completed Nov 12, 2021
@vaitkus
Copy link
Collaborator Author

vaitkus commented Nov 15, 2021

I reopen this issue due to the comments from @jcbollinger on issue COMCIFS/DDL1-legacy-dictionaries#5 [1]. Relavent comment:

It may be that some of these items instead should not be afforded the possibility of an SU in the DDLm core. In addition to the diffrn_refln_counts* items, I am looking at diffrn_standards_scale[sigma, u] and refine_ls_restrained_s[all, obs, gt].

Also, although atom_sites_cartn_tran_matrix[1, 2, 3][1, 2, 3], atom_sites_cartn_tran_vector[1, 2, 3], atom_sites_fract_tran_matrix[1, 2, 3][1, 2, 3], and atom_sites_fract_tran_vector[1, 2, 3] are derived from cell parameters, many of which do have standard uncertainties, I think there are circumstances under which the transformations described are meant to be or should be treated as exact. At minimum, one needs to recognize that if they are treated as inexact transformations then they are irreversible with respect to their effects on SUs.

[1] COMCIFS/DDL1-legacy-dictionaries#5 (comment)

@vaitkus vaitkus reopened this Nov 15, 2021
@jamesrhester
Copy link
Contributor

  1. _diffrn_standard_scale_sigma:

As this value is a straightforward average, an SU can be calculated, however it seems supremely pointless so I would be happy to drop it.

  1. _refln_ls_restrained_s

I have a memory of seeing SU attached to goodness of fit parameters. While their usefulness may be doubtful, I think we need to allow the SU to be stated if there is software that wants to do it.

  1. _tran_matrix,_tran_vector

I think it is an inescapable fact that these matrices are not exact. We could add something in the definition noting that these matrices are not exact, although that is already contained in the machine-readable properties.

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