FAIRmat-NFDI / nexus_definitions

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

Define usage of NXtransformations and NXcoordinate_system(_set) #144

Closed lukaspie closed 2 weeks ago

lukaspie commented 6 months ago

Fixes #143.

EDIT: an example for converting from one CS to another by using NXcoordinate_system/(NXtransformations) can be found here.

lukaspie commented 5 months ago

@sanbrock, @rettigl, and I had another discussion today and came up with the following idea.

For the future, @sanbrock suggested another option: add everything that is now in NXcoordinate_system to NXtransformations, thus making it possible to describe non-affine transformations and changes in handedness by NXtransformations. In that case, we would add the following to NXtransformations:

NXtransformations:
  BASE_COORDINATE_CHANGE(NX_NUMBER):
    @x(NX_NUMBER):
    @y(NX_NUMBER):
    @z(NX_NUMBER):
    @offset(NX_NUMBER):

What do you think @FAIRmat-NFDI/areab?

tomio13 commented 5 months ago

Do I understand it correctly, that with this you stuffed an NXtransformation into the NXtransformation, the former one forcing a trasnlation shift along a vector, the second being whatever usual transformation (translation or rotation) it would have anyway been?

lukaspie commented 5 months ago

Do I understand it correctly, that with this you stuffed an NXtransformation into the NXtransformation, the former one forcing a trasnlation shift along a vector, the second being whatever usual transformation (translation or rotation) it would have anyway been?

No, there is no nesting of NXtransformations. The idea (please correct me if I am wrong @mkuehbach) is that in NXcoordinate_system, you have three base vectors that define this coordinate system with respect to the CS that you refer to with the @depends_on attribute. This would be a passive transformation.

Then, any AXISNAME in NXtransformations can point (via @depends_on) to one of the NXcoordinate_system instances that you define somewhere else in the tree and then you know that this transformation lives within that CS.

The main idea why we want to use NXcoordinate_system (instead of describing the CS also with NXtransformations) is that with the existing NXtransformations base class, it is not possible (to my understanding) to describe changes in handedness and non-affine transformations during a CS change.

tomio13 commented 5 months ago

So, basically complement the NXtransformations with a coordinate system defined by 3 base (unit) vectors. With this, transferring the so-far free text definition to this base class. The base vectors can be right-handed or left handed, and the vector in the NXtransformation is then meant to be in this coordinate system.

mkuehbach commented 5 months ago

NIAC conceptualized that NXtransformation can be connected into a graph with one single-directed edge between two nodes (NXtransformation) instances (a typical triplet). There is no parent-child relationship implied via this conceptually, therefore yeah no nesting of NXtransformations.

Now NXcoordinate_system and NXtransformations: What I proposed is indeed NXcoordinate_system as an explicit way to say I have that many coordinate systems with the i) possibility to define their base vectors explicitly as childs of respective NXcoordinate_system instance see fields x,y,z, ii) lately the connection between NXtransformations and NXcoordinate_system has been discussed rightly so, I support to define base vectors as childs in an NXcoordinate_system instance but then there have to be three disjoint NXtransformations instances one for each base vector (if parallelepiped is to be spanned).

In my opinion another possibility to achieve the same thing: State explicitly what your coordinate system is. depends_on as an attribute of NXtransformations is reserved to chain NXtransformations, not to connect into NXcoordinate_system, however, I am not happy with this because stating that . is the end of a transformation chain also means where is this chain defined in, if nothing is stated McStas. But what if I have a community which does not want to use McStas in this case I see not a problem with the thought of using a triplet of NXtransformations inside NXcoordinate_system but than this triplet cannot be the end point as that is a chain with say one NXtransformation out and three in or vice versa, leaving an ill-defined state. But why can one not set an attribute the specifies explicitly in which CS an NXtransformation is living, i.e. I do not really see why one must not use depends_on to build a directed graph between an instance of NXcoordinate_system and an instance of NXtransformations.

lukaspie commented 5 months ago

i) coordinate systems with the i) possibility to define their base vectors explicitly as childs of respective NXcoordinate_system instance see fields x,y,z, ii) lately the connection between NXtransformations and NXcoordinate_system has been discussed rightly so, I support to define base vectors as childs in an NXcoordinate_system instance but then there have to be three disjoint NXtransformations instances one for each base vector (if parallelepiped is to be spanned).

This is what I don't understand about NXcoordinate_system. From what you said here, I can (arbitrarily) define three vector x, y, and z, but how do I say with respect to what I define these? Would they not have to explicitly defined w.r.t. to McStas or another CS? My idea was that you define these vectors and use the depends_on attribute of NXcoordinate_system to say what these vectors are in another CS.

But why can one not set an attribute the specifies explicitly in which CS an NXtransformation is living, i.e. I do not really see why one must not use depends_on to build a directed graph between an instance of NXcoordinate_system and an instance of NXtransformations.

We also discussed this. The alternative would be to define a new attribute in NXtransformations (how about @coordinate_system) to explicitly define in which CS this particular NXtransformation is living.

rettigl commented 5 months ago

For orthonormal coordinate systems, one can can discribe it simply with one or more NXtransformation matrices, and the new base vectors are the MCstas base vectors times the resulting matrix, no? This is the use case I need. My idea is to create the NXcoordinate instance, and define the depends_on and NXtransformation chain within to define this matrix (even not give the vectors, as these result from the transformations).

depends_on as an attribute of NXtransformations is reserved to chain NXtransformations, not to connect into NXcoordinate_system

"." itself is not a transformation, but rather a coordinate system, so I would say we should extend the use of @depends_on to allow to point to other NXcoordinate system instances.

sanbrock commented 5 months ago

Discussing it with NIAC yesterday, it is indeed supported to extend NXtransformatinos so it does not only support rotation and translation but also "general" transformation described by an arbitrary transformation matrix (in fact x,y,z of our coordinate_system does describe such matrix by providing its column vectors). Hence, a base change is just yet another transformation type which can be part of the @depends_on chain wherever it is needed (not only at the end; and not only at a single point in the chain).
Note that CIF (which the idea of NXtransformation was built on) does have that capability and also supports working with fractional coordinates. It was also suggested to extend the dimensionality of NXtransformations to be able to support more dimensions than just 3. This could be implemented by introducing a symbol (e.g. 'n') representing the rank, so vectors shall follow that. In a coordinate transformation, we either give an n x n matrix and a translation vector, or directly an n+1 x n+1 augmented affine transformation matrix which can do translation, rotation, but also reflection, scale, and shear.

mkuehbach commented 5 months ago

Much of the last three comments I second. One point I would like to make more clear: It is alright that McStas is a convention some people wish to use. Fact is that it is one of many possible ones and NXcoordinate_system is an attempt to show that NeXus user are not necessarily required to use McStas if they dont want to.

On @lukaspie your point where is an instance of coordinate system embedded in: that is to some extend always arbitrary at the origin of the chain. A map of the globe is in world coordinate system where is that embedded in a cosmology-based reference frame but where is that embedded in... exactly it is defined as is. Therefore, we have the free-text fields in like "edge of the table etc".

Now on generalizing NXtransformation: Yeah that is probably a useful strategy for the future. The definition of say base vectors (vectors in general in maths) is within a reference frame, say we define a vector as a tuple of numbers in $\mathcal{R}^d$, again where is $\mathcal{R}^d$ defined in, the discussion ends as aforementioned. You define a few rules and then use only this ruleset to derive everything else. Conceptually we may ask is the concept of a transformation and a reference frame exactly the same. I argue no and that is exactly reason i) why I proposed NXcoordinate_system*. Reason ii) was that for many practical cases the discussions here are already way to detailed and not only from a political point we should first of all convince people to document their reference frame conventions at all and doing so explicitly. Ones this has been achieved with either transformations or coordinate systems, the discussion here is relevant. Then I think we have to make a difference between the concept of a reference frame and the concept of transformation happening within these reference frames or affecting the orientation of these reference frames.

rettigl commented 3 weeks ago

@lukaspie What is the status of this? As both NXxps and NXmpes_arpes use the transformations in NXcoordinate_system, I would suggest to merge this now (however, rn the transformations are actually not included!)

lukaspie commented 3 weeks ago

@lukaspie What is the status of this? As both NXxps and NXmpes_arpes use the transformations in NXcoordinate_system, I would suggest to merge this now (however, rn the transformations are actually not included!)

My plan was to present/discuss this tomorrow in the TF meeting. @sanbrock actually had another idea how we could change NXtransformations, so we can just discuss it all together and then merge this PR afterwards.

EDIT: I added (NXtransformations) and depends_on to NXcoordinate_system again as discussed in the description of the PR.

lukaspie commented 2 weeks ago

Feedback from today's TF meeting:

@rettigl based on these discussion in the TF, I would say we can merge this PR now and then figure out all the new discussions in #235. Do you agree?