Fix coord conversion position unit mismatch exceptions#633
Fix coord conversion position unit mismatch exceptions#633SimonHeybrock merged 4 commits intomainfrom
Conversation
There was a problem hiding this comment.
I think it's fine to return a fixed unit. We do the same in other places, too. E.g., to ensure that wavelengths are in Å.
| Compute the incident beam for a straight beamline. | ||
| """ | ||
| return sc.norm(incident_beam) | ||
| return sc.norm(_canonical_length(incident_beam)) |
There was a problem hiding this comment.
Can you apply it to the result of norm? That uses 3x less memory if it has to convert.
| ``scattered_beam`` | ||
| """ | ||
| return position - sample_position | ||
| return _canonical_length(position) - _canonical_length(sample_position) |
There was a problem hiding this comment.
Here, you convert the inputs. In straight_incident_beam, you convert the output. Is there a reason? I think since the positions are typically read from file, the implementation in straight_scattered_beam makes more sense.
There was a problem hiding this comment.
Good catch, need to fix straight_incident_beam to convert individually, or else we may get more exceptions.
| This is used in reflectometry. | ||
| """ | ||
| incident_beam = _canonical_length(incident_beam) | ||
| scattered_beam = _canonical_length(scattered_beam) |
There was a problem hiding this comment.
This should not be necessary. If it is, then there is something wrong with _scattering_angles_with_gravity_generic or _scattering_angles_with_gravity_orthogonal_coords which currently convert dtypes but not units.
There was a problem hiding this comment.
It was necessary to fix a failing unit test with gravity.
Issues seen during DREAM data reduction.
Note sure converting everything to meter is nice in all cases, but everything else felt like a mess and totally unpredictable.