Update FormatSMVRigakuSaturn to recognise multi-axis crystal goniometers #617
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #617 +/- ##
==========================================
- Coverage 39.43% 39.38% -0.06%
==========================================
Files 178 178
Lines 15475 15495 +20
Branches 2603 2613 +10
==========================================
Hits 6103 6103
- Misses 8812 8832 +20
Partials 560 560 |
There was a problem hiding this comment.
Some suggestions inline but nothing serious
Only question I have is do you have any non-trivial test data e.g. where the scan axis is phi or anything? If you have an instrument could be worth collecting something with an unusual set of axis configurations just to make sure it does what you expect away from datum positions.
Otherwise change set looks good, efforts appreciated.
I don't have access to such an instrument or dataset: all I have currently is the dataset referenced in #616 (omega scanning but non-default chi axis for some scans) which I plan to check using the new checkCIF for raw data tool after generating cbfs via dials. I figure if everything is correct after that workflow then the bulk of cases for this type of instrument are covered. |
|
Hi James, are you happy to be credited in the AUTHORS and CHANGELOG? |
|
Yes, can credit me, thanks. |
|
Given that Graeme had a couple of outstanding questions, I'm not going to merge this in today for DIALS 3.14. However, I'm happy for us to include it in a patch release because it's fixing an actual bug. |
There is no need for a special case for a single-axis goniometer.
…xtbx into multi-axis-saturn
|
As ever, time marches on faster than I can track it. Merging this in for 3.15 instead. |
…x#617) FormatSMVRigakuSaturn class previously assumed a single-axis crystal goniometer. When there are multiple axes, and these are at non-zero positions during a scan, inter-scan relationships will be incorrect. Copies the code already present for multi-axis crystal goniometers in FormatSMVRigakuEiger and add a check for a single axis, in which case the previous behaviour is preserved. Closes cctbx#616 .
As described in #616 , the
FormatSMVRigakuSaturnclass assumes a single-axis crystal goniometer. When there are multiple axes, and these are at non-zero positions during a scan, inter-scan relationships will be incorrect. This PR simply copies the code already present for multi-axis crystal goniometers inFormatSMVRigakuEigerand adds a check for a single axis, in which case the previous behaviour is preserved.Closes #616 .