Enet4 / nifti-rs

Rust implementation of the NIfTI-1 format
Apache License 2.0
41 stars 11 forks source link

Unused dimension 0 or 1 #84

Closed nilgoyette closed 3 years ago

nilgoyette commented 3 years ago

We were looking at the nifti specification to know if we should write

dim = [N, w, h, d, 0, 0, 0]
OR
dim = [N, w, h, d, 1, 1, 1]

but it's not entirelly clear! The standard says

dim[i] = length of dimension #i, for i=1..dim[0]  (must be positive)

Is zero positive or negative? Such is the question :) A quick research returns a math.stackexchange page. It seems that there a difference between "positive" and "strickly positive", but the standard doesn't use the clear term "strickly positive"... 0 seems to be both "positive" and "negative" AND neither, depending on who you ask, the context and the position of Venus.

What do we have in nifti-rs?

What do we do with this? It's not a bug, nifti-rs still works as intended, but... do we "fix" it?

Enet4 commented 3 years ago

Unless I overlooked something, I think that we would be safe to keep using dim as we are now. Here is my rationale:

I believe that was originally intended in NIfTI-1 from the start was that implementers:

  1. Must only read the dimensions in dim from dim[i] up to dim[0], and reading beyond that is a bug that could lead to ill-formed behavior;
  2. May choose to initialize the remaining unused values to 0 or 1, but only for convenience. Even the header file at some point suggests that "It is best to initialize ALL fields in the NIFTI-1 header to 0 (e.g., with calloc()), then fill in what is needed."

We are good with the first rule, and for the creation of fresh new volumes we should be good with the second one too. In a similar fashion, it's faster to just copy a [u16; 8] into a Dim or Idx without cleaning up the unused values. The only exception I can think of from the second rule would be if we stumble upon a... creative implementation that relies on these unused parameters to be set to something reasonable. Should something like that come around, we would then assess the implications of making the library more compatible with that implementation.

nilgoyette commented 3 years ago

Ok, perfect, thank you for your input. It was useful at least to discuss it and to make things clearer.