-
Notifications
You must be signed in to change notification settings - Fork 1
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
Mcstas instrument geometry in the loader. #18
Conversation
The instrument view is inserted as a code-snippet not as a cell.
Here is the screen shot of the instrument view: |
src/ess/nmx/mcstas_xml.py
Outdated
theta, x, y, z = find_attributes( | ||
location, 'rot', 'axis-x', 'axis-y', 'axis-z' | ||
).values() | ||
q = axis_angle_to_quaternion(x, y, z, sc.scalar(-theta, unit=angle_unit)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using keyword args would avoid potential bugs in functions like this with many similar arguments.
src/ess/nmx/mcstas_xml.py
Outdated
def _rotate_axis(matrix: sc.Variable, axis: sc.Variable) -> sc.Variable: | ||
return sc.vector(np.round((matrix * axis).values, 2)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do? Why are we rounding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Justin said that's the precision that is needed so I kept it here.
Hmm... but I will remove rounding here so that it is handled in the next step if necessary.
src/ess/nmx/mcstas_xml.py
Outdated
Position of each pixel is relative to the position_offset. | ||
""" | ||
pixel_idx = sc.arange('id', detector.total_pixels) | ||
n_row = sc.scalar(detector.num_fast_pixels_per_row) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this n_col
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah actually column is more correct.
'pixel_ids': _construct_pixel_ids(self.detectors), | ||
'fast_axis': sc.concat(fast_axes, detector_dim), | ||
'slow_axis': sc.concat(slow_axes, detector_dim), | ||
'origin_position': sc.concat(origins, detector_dim), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the origin of each panel?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it is. The position of the first pixel (lowest pixel ID).
I couldn't come up with a better name...
src/ess/nmx/mcstas_xml.py
Outdated
def _rotate_axis(matrix: sc.Variable, axis: sc.Variable) -> sc.Variable: | ||
return matrix * axis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helper seems pointless now, just inline the multiplication?
Co-authored-by: Simon Heybrock <[email protected]>
b36dd6a
to
f4495e2
Compare
Fixes #10
Continued from #15
Now all fields that could be derived from the geometry xml description are included as coordinates of the
NMXData
(the output type of the workflow.)These coordinates cover most of the output fields that need to be saved as an intermediate result for #16