Skip to content

Alter category key pd_calib_xcoord #315

Open
rowlesmr wants to merge 14 commits into
COMCIFS:masterfrom
rowlesmr:rekey-PD_CALIB_XCOORD-take-2
Open

Alter category key pd_calib_xcoord #315
rowlesmr wants to merge 14 commits into
COMCIFS:masterfrom
rowlesmr:rekey-PD_CALIB_XCOORD-take-2

Conversation

@rowlesmr

@rowlesmr rowlesmr commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

pd_calib_xcoord - originally keyed on .id and .xcoord_overall_id (linked to _pd_calib_xcoord_overall.id). There was no way to link each point of the calibration to the detector it was calibrating. .xcoord_overall_id was demoted to linked non-key data item. .detector_id promoted to part of composite key. Updated the examples.

@jamesrhester

Copy link
Copy Markdown
Contributor

Just making sure I understand the logic here. An x-calibration, described in PD_CALIB_XCOORD_OVERALL has a globally unique identifier which identifies a diffractogram and phase used for the calibration. As neither phase nor diffractogram are keys, multiple calibrations could be listed that use the same source diffractogram and phase but go about it slightly differently. The particular calibration used is therefore indicated in the _pd_calib_xcoord.xcoord_overall_id data name. Via this calibration measurement we have the instrument and detector that is the target of calibration, so doesn't that mean we already have the detector_id without needing to restate it? I believe a detector should never be calibrated using a measurement from a different detector?

@rowlesmr

rowlesmr commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator Author

The way I see it is that pd_calib_xcoord and pd_calib_detected_intensity (both Loop categories) should share exactly the same structure, as they are doing fundamentally the same thing: calibrating a detector, somehow.

So detector_id should be a key. I've become not a fan of long chained lookups, as it is too easy to lose a link in that chain. The aim of this PR was to provide a direct linkage, rather than a chained lookup.

Indeed, pcdi is keyed on detector_id, and the "mandatory" value is detector_response. You can optionally have values for diffractogram_id, phase_id, and special_details to give details about how the calibration was done. These could have been broken out into a pd_calib_detected_intensity_overall Set category, but the number of detectors is normally low, so it didn't seem an imposition to write everything for each line.

In this proposed PR, pcx is keyed on detector_id, as we're calibrating a detector, and on id, as we need to calibrate many positions per detector. The (at least) two "mandatory" values are nominal_thing and actual_thing. You can optionally detail the diffractogram, phase, or special details behind the calibration, by providing those in the Set category pd_calib_xcoord_overall, linking those details to each loop packet in pcx.

@jamesrhester

Copy link
Copy Markdown
Contributor

Chained lookups are meant to be a feature of a properly normalised relational model, in that the information about something is always bundled behind its identifier (or multiple identifiers if it's a joint property) instead of information being repeated more than once and creating the danger of inconsistent information. But there are different ways to skin that relational cat, depending on taste. What we do want to avoid is any column not being a function of all the key data names, or in other words, it shouldn't be possible to determine the value of a data name in some category without knowing the value of every key data name in that category, because then that data name is more properly describing a property of the key data name that it depends on. What you win with all this chaining is the ability to make localised changes without affecting other categories.

Anyway, I agree that the second key data name in pcx could be either _pcx.detector_id or _pcx.xcoord_overall_id.

Let's think this through. This PR implicitly assumes that there is a unique X calibration for a detector in a data set, as knowing the detector and calibration point is enough to determine the new X coordinate (this agrees with the intensity calibration approach, which also assumes a single intensity calibration.) Therefore, the overall X calibration information is a function of the detector and we can define non-key data name _pd_detector.overall_xcoord_id pointing to the overall X-calibration information.

Furthermore, a consequence of changing the key to _pcx.detector_id is that _pcx.xcoord_overall_id (besides it needing to be explicitly specified in each row) is a function of only one of the key data names. We should therefore delete this data name as it violates the rule I put forward in the first paragraph.

So I'd be happy with this PR if _pd_detector.overall_xcoord_id is replaced by _pd_detector.xcoord_overall_id.

@rowlesmr

Copy link
Copy Markdown
Collaborator Author

I'm kind of following you right up to the last line. Those last data names make no sense.

@jamesrhester

Copy link
Copy Markdown
Contributor

Whoops, that last line should read:

So I'd be happy with this PR if _pd_calib_xcoord.xcoord_overall_id is replaced by (new data name) _pd_detector.xcoord_overall_id.

@rowlesmr

Copy link
Copy Markdown
Collaborator Author

I reckon I understand that. The principle re a function of all keys drives it home.

Will make those changes tonight.

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

Successfully merging this pull request may close these issues.

2 participants