nexusformat / definitions

Definitions of the NeXus Standard File Structure and Contents
https://manual.nexusformat.org/
Other
26 stars 56 forks source link

missing default @vector values for fast/slow_pixel_direction #1247

Open jkotan opened 1 year ago

jkotan commented 1 year ago

In https://github.com/nexusformat/definitions/issues/1211 there is a discussion between @PeterC-DLS, @zjttoefs and @wakonig about fast_pixel_direction, slow_pixel_direction and module_offset. In that issue

          Usually, the (0,0) pixel is at the upper left corner when viewing the detector face from the sample. If the detector is not in this orientation then use `NXdetector_module` and its `fast/slow_pixel_direction` fields to specify it.

Originally posted by @PeterC-DLS in https://github.com/nexusformat/definitions/issues/1211#issuecomment-1271356261

Does it means @vector attributes of fast/slow_pixel_direction have defined default values i.e.

   slow_pixel_direction
     @vector   = [0, -1 ,0]

   fast_pixel_direction
     @vector   = [-1, 0, 0]

where minuses come from an opposite direction with respect to NeXus coordinate system or I over-interpret that words?

I cannot find these default values in the documentation so if it is true I think it should be added.

There is also interesting discussion at https://github.com/nexusformat/definitions/issues/412 especially https://github.com/nexusformat/definitions/blob/main/manual/source/_static/412-detector-docs/detectors.txt Maybe it would be good to verify it and add it.

PeterC-DLS commented 1 year ago

Hi Jan, yes, those directions are correct. They probably should be specified in the document for NXdetector as applying when there is no NXdetector_module.

jkotan commented 1 year ago

Hi @PeterC-DLS , thanks for your fast answer. I have two other questions related to NXdetector_module class. 1) Shall the @vector attribute of module_offset,fast_pixel_direction and slow_pixel_direction be normalized to 1? At https://github.com/nexusformat/definitions/blob/e9f7406b252a4b209bb50da830e2d27c7f280c9c/base_classes/NXdetector_module.nxdl.xml#L75-L79 there is nothing mentioned about the normalization. However, for a similar NXtransformations/@vector at https://github.com/nexusformat/definitions/blob/e9f7406b252a4b209bb50da830e2d27c7f280c9c/base_classes/NXtransformations.nxdl.xml#L141-L154 the @vector is normalized to unit length. 2) In https://github.com/nexusformat/definitions/blob/e9f7406b252a4b209bb50da830e2d27c7f280c9c/manual/source/_static/412-detector-docs/detectors.txt#L120-L125 i.e. detector_module_1 there is introduced a "detector coordinate system" to my understanding with flipped the x-axes with respect to the NeXus coordinate system. Reading on https://github.com/nexusformat/definitions/blob/e9f7406b252a4b209bb50da830e2d27c7f280c9c/manual/source/_static/412-detector-docs/detectors.txt#L170-L184 module_offset is defined in this "detector coordinate system" contrary to slow_pixel_dimension. So the question is if it is a mistake or module_offset should be defined in the "detector coordinate system" e.g. without negative coordinates?

PeterC-DLS commented 1 year ago

I think it is assumed that as we named them *_pixel_directions then they are direction vectors and should be normalized. Thus their value is the pixel size/spacing in the given direction.

module_offset is the offset from the detector origin in the same frame as how the pixel directions are defined. The detector's local coordinate frame is defined by its dependency chain (and not by its pixel directions). Thus module_2/module_offset@vector should be [-0.6, 0.8, 0].

~I think module_*/data_origin fields are incorrectly transposed and, e.g., module_2/data_origin should be [8,6]~

It seems to me that this document is a little confusing in its introduction of a new xyz coordinate frame instead of keeping it simple with fast/slow directions. The contributors @prjemian , @phyy-nx , @yayahjb can confirm whether the example conforms to the standard.

soph-dec commented 7 months ago

I would also be interested in what conventions should be used for the coordinates given in an NXdetector_module.

For all the fields in question, namely module_offset, fast_pixel_direction and slow_pixel_direction, there is an optional depends_on attribute. Where should this point to? If the vector attribute is given in "lab coordinate system" (NeXus coordinate system with origin in the sample), then there is no need for the depends_on, or am I misunderstanding something? Or we give the vector in the "detector coordinate system" (with origin on the top left corner and x and y in opposite directions of the NeXus coordinate system) and use the depends_on to point to the transformation chain in the detector group that describes the coordinate transformation from detector to lab coordinates.

In addition, there is an optional depends_on field for the whole NXdetector_module group, how does this combine with the depends_on attributes of the fields in question? Also, the NXdetector_module group is usually a child of an NXdetector group, which has an optional depends_on field as well. Do child groups and fields inherit depends_on information from parent groups? What if both are given?

PeterC-DLS commented 7 months ago

The depends_on attribute for the pixel directions could point to module_offset and module_offset could depend on the same transformation as the enclosing NXdetector_module group's depends_on field.

Maybe this could be the procedure:

  1. if the field is defined, the attributes could be optional and the three directions are relative to that dependency
  2. if the field is not defined, module_offset's depends_on must be defined and the pixel directions considered relative to module_offset
soph-dec commented 7 months ago

Thank you for the quick reply!

If I understand correctly, that means that the depends_on field of the parent group is not relevant for a child group, but for the fields contained in the (parent) group it is, regardless of if they have their own depends_on attribute set. But then there is no option to "override" a group's transformation chain for a single field in that group. I would have expected to use the attribute if it is set and otherwise the "global" transformation defined in that group. But then again, I would also have expected to extend to parent groups. I feel like there is some more clarification needed in the documentation for this.

The depends_on fields define where the chain of transformations start. If my understanding is correct, this is used to "place" a piece of equipment in/during the experiment. If that is correct, then the pixel directions don't add to that information, only the module_offset does. Then one would maybe expect the depends_on of the NXdetector_module to point to module_offset, but the pixel directions would not be part of that chain. (Unless those directions can be used to implicitly define a rotation. Is that possible? Do different NXdetector_modules have different pixel directions?)

Thinking further about it, with the depends_on attribute one is able to clarify what coordinate system is used, for example if a direction were given in "detector coordinates", it would make sense to point to a transformation in the detector group (the same one that the depends_on field of the NXdetector group points to). If it is given in "lab coordinates", the depends_on attribute would simply be ".". If that is correct, it would mean that one can choose the coordinate system used for the pixel directions.

PeterC-DLS commented 7 months ago

I think I got it wrong with the depends_on field - however NXdetector_module is a special case as it's not a NXtransformations group and the pixel directions are not transformations per se.

My best guess is that this group's depends_on field could point to module_offset and both pixel directions should depend on module_offset. I.e. the depends_on field may be redundant in this case. In my parsing code, I traverse the dependency chains of the three fields (module_offset and *_pixel_direction) separately.