Repository navigation
Conversation
github-actions
Bot
changed the title
| def _orientation_axis( | ||
| transform: sc.Variable | sc.DataArray, axis: tuple[int, int, int] | ||
| ) -> sc.Variable: | ||
| if isinstance(transform, sc.DataArray) or transform.ndim != 0: |
There was a problem hiding this comment.
Is a log with just one value allowed? It would still have a dimension but a length of 1 in that dimension.
There was a problem hiding this comment.
I agree that it's strange that this function is somehow "responsible" for handling the potential time dependence. I think it would make most sense to just make it agnostic to the dimensions of the input.
I don't think that makes sense to specifically accept scalar or length one. Because length one doesn't mean the same as scalar. Scalar means "time independent", but length one means "time dependent, unknown before timestamp and constant after".
There was a problem hiding this comment.
In the DiskChopper constructor, we allow both scalar and length 1.
That's why I brought this up here. But fine if you'd rather keep the distinction clear between the two.
|
|
||
| Use the same composed, time-filtered transform as the detector positions. | ||
| """ | ||
| direction = _orientation_axis(transform.value, (0, 0, 1)) |
There was a problem hiding this comment.
It seems from the docstring above that having z as the direction here while it is x in load_sample_rotation is intentional, but I'm just double-checking that it was not a typo.
Maybe expanding the docstring a little with a small explanation would help?
| roll = sc.spatial.rotations_from_rotvecs(sc.vector([0, 0, 45], unit='deg')) | ||
| translation = sc.spatial.translation(value=[123, 0, 0], unit='mm') | ||
| transform = NeXusTransformation(yaw * translation * pitch * roll) | ||
| # For the sample surface, yaw = 1.25 - atan(sin(30) * tan(45)). |
There was a problem hiding this comment.
Did I understand correctly that this is where the number -25.3150511771 comes from?
There was a problem hiding this comment.
Yes you're right. But I removed this test, it didn't seem very useful.
The Estia workflow previously hard coded how to read sample orientation and detector orientation from a log, this PR makes it use the transformations derived from the nexus structure instead.