openMetadataInitiative / openMINDS_SANDS

A metadata model capturing Spatial Anchoring of Neuroscience Data Structures, including brain atlas definitions.
MIT License
12 stars 11 forks source link

schema-revision: options/axes.schema.json #35

Closed lzehl closed 3 years ago

lzehl commented 4 years ago
  1. origin and directions should be an array of numbers no?
  2. if anatomicalLables is the semantic version of directions do we also need one for origin?
UlrikeS91 commented 4 years ago
  1. What was directions supposed to be again? (1 0 0), (0 1 0) and (0 0 1)? origin should be an array of number, yes. Also as a general comment to that (since it applies to several schemas): We should limit the items of the array to 3, right? Like this: { "type": "array", "minItems": 3, "maxItems": 3 } Or are there cases where this is not true and we need fewer/more items?

  2. I would say no. anatomicalLabels is the semantic version of directions yes, but I don't think this applies to the origin. Terms like "ventral", "dorsal", "medial", "temporal" cannot be applied to an origin, but could be in relation to the origin (as in "ventral to the origin").

UlrikeS91 commented 4 years ago

Follow-up question (maybe a stupid one...): The indexingParameters have units for the axesMin and axesMax, do we need units in the axes-schema then? Do directions need units?

lzehl commented 3 years ago

summary of properties:

@UlrikeS91 this one needs again a bit of input / clarification first.

xgui3783 commented 3 years ago

@lzehl Can I clarify that axes.schema.json is meant to represent all axes in a coorindate space?

origin (array of integers (0 to 1), items count:3) correct ??? what is assumed here as 0, 0 , 0 (right-handed Cartesian coordinate system ? this is unitless correct?)

I struggle to understand why origin is needed at all. I would have understood the origin is by definition [0,0,0] in a 3D space. If a non 0 value is provided, then one must ask the question, the origin is [20, 20, 20], relative to what.

units (controlledTerm, count: 1) ( if origin and direction are unitless and units are defined in indexingParameters is unit here needed?)

This is something which I will need input from @skoehnen . From the indexingParameter issue, I have expressed by concerns for exposing the indexingParameter in SANDS. Even if we put that aside, IMO, unit is a property of axes, and not the other way around. (indexingParameter can be unitless in that case, inheriting/inferring the unit from axes)

mathematicalDirections (array of integers (0 to 1), items count: 3, array count: 3) correct ??? this is unitless correct?

I can't find reference to mathematicalDirections anywhere. to me, anatomicalDirections is sufficient, and makes supercedes mathematicalDirections

lzehl commented 3 years ago

Yes! thanks that you brought this up. I did not understand this part as well completely, because it originates from some discussions we had with Lionel, way back...

1) yes, axes.schema.json was now meant to represent ALL axes Feel free to make a counter suggestion if you think each axes should be represented individually

2) I (and Ulrike) have the same question about the origin... I assumed that the relation was always to the technical origin of images (upper, left front corner is [0,0,0]?)

3) good to hear your feedback on indexingParameters and axes. Does this mean: a) unit will be property of axes b) should minAxes, and maxAxes also be part of axes? c) steps in xyz should be defined in indexingParameters d) what else is needed in indexingParameters?

xgui3783 commented 3 years ago

Feel free to make a counter suggestion if you think each axes should be represented individually

Now that i step back a bit, I think it does make sense to have the axes represented in a single object.

I assumed that the relation was always to the technical origin of images (upper, left front corner is [0,0,0]?)

Then shouldn't it reside in a description field of some kind? something like

{
  "@type": "https://schema.ebrains.eu/sands/Axes",
  "description": "This axes describes the histological scan of Big Brain 20, slice 1336. The origin, per computer image standard, Is the top left most point of the image"
}

Or would you prefer to have a number of controlled_terms for common origins? e.g. 2D image (top left), brain atlas (somewhere in the middle) etc?

a) unit will be property of axes

this indeed is my opinion

b) should minAxes, and maxAxes also be part of axes?

Hmm, I think I remember this now. I requested this to be placed in, because I needed to convert coodrinates from PIR to RAS. I don't know the answer to this question yet. Imposing a minAxes/maxAxes seems strange (afterall, axes, by definition, should extend indefinitely). I will perhaps have to thinking about this

c) steps in xyz should be defined in indexingParameters d) what else is needed in indexingParameters?

Hmm, if they need to be somewhere, they will have to be in indexingParameters. Steps in Axes wouldn't make sense (unless we are talking about voxel/pixel resolution). But it should not affect users from placing a landmark between two steps.

I am still under the impression that indexingParameter should not be public facing, but I think we should reserve that discussion in person/on zoom.

lzehl commented 3 years ago
lzehl commented 3 years ago
xgui3783 commented 3 years ago

@skoehnen I still struggle to understand how directionsMatrix solves the issues of:

"origin" needs to be clearly defined (origin in relation to what?)

I wonder if we need to define where the origin is, rather than how it is defined. Consider nifti transform header. It instructs programs how to find the origin, without explaining if the origin has any meaning what-so-ever.

For our purpose, can we not just say:

sands/core/coordinateSpace/v1.0.0/bad-b01-1

{
  "origin": [10, 20, 30],
  "unit": "mm",
  "anatomicalOrientation": "RAS",
  "defaultImage": "sands/core/image/v1.0.0/bad-b01-2"
}

Then, in principle, we find the most left, most posterior, most inferior voxel fo the image, and count 10 units to the right, 20 units towards the anterior, and 30 units towards the superior, and it would be our origin?

lzehl commented 3 years ago

@skoehnen this sounds like it would work. Would it work for all cases (all anatomicalOrientations?) besides RAS?

lzehl commented 3 years ago

feedback from SANDS meeting:

axes.schema.json
    anatomicalOrientation [expects: controlledTerm; count: 1]
    origin [expects: number; count: 3]
    unit [expects: controlledTerm; count: 1]

In addition: please fix $ref of AXES in defintions.json (elements->options) [ @xgui3783 thanks :wink: ]

@UlrikeS91 could you take this revision?

lzehl commented 3 years ago

merge into #40