fNIRS / snirf

SNIRF Format Specification
http://fnirs.org/resources/software/snirf/
Other
59 stars 34 forks source link

landmark and landmarkName likely shouldn't be part of aux ? #8

Closed dboas closed 5 years ago

dboas commented 5 years ago

Qianqian, Jay, Meryem and I think instead of aux.landmark and aux.landmarkName should be sd.landmark and sd.landmarkName Are you okay with that?

fangq commented 5 years ago

in Sungho Tak's email, he asked for storage of two datasets: 1) 3D digitizer data and 2) landmark (10-20) positions, likely also acquired from the digitizer.

in my opinion, landmark/landmarkName are places to store digitizer data, which can be independent to source detector placement - for example, it may be point-cloud to record head contour, or EEG landmarks that are not necessarily part of optical probe. So, I felt that aux is the better place. But I am happy to follow your preference as I haven't processed any real fNIRS data myself.

fangq commented 5 years ago

hi @jayd1860, regarding your questions

a) I was wondering about the landmarks fields in aux. Since I'm not an experimenter I was curious how they might be used.

I thought the aux.landmark stores the array of position data acquired from the digitizer . For example, if

aux.landmark=[
10.3 11.0 5.3 1
22.2 12.0 1.5 2
11.2 19.1 9.1 3
 2.2  4.0 7.2 4
18.2  2.0 5.2 4
];

aux.landmarkName=[
"Nasion",
"Inion",
"Cz",
"Undefined"
];

the above data stores 5 digitizer coordinates, the last column map them to the landmarkName - i.e. the first position, [10.3 11.0 5.3] is the Nasion position, and so on.

b) In the middle of the aux parameters we have timeOffset. Should I move it to be after all the aux parameters?

I see timeOffset is located at the end of the aux, are you sure it is in the middle? which version are you looking at?

c) This is a minor stylistic question. I noticed under sd the following fields weren't in bold like the rest.

sd.frequency sd.timeDelay sd.timeDelayWidth sd.momentOrder

Should I make them bold like the rest?

sure, if you prefer.

fangq commented 5 years ago

The current definition does not restrict the maximum landmark field, so that we can store more than 4 columns, for example

x, y, z, id, nx, ny, nz, status ...

where the x, y, z, nx, ny, nz, status are direct output of the Polhemus digitizer when you press the stylus button. I currently leave the column names user-defined, but we can certainly add aux.landmarkColumnName to specify the meaning of each column.

dboas commented 5 years ago

Qianqian, landmarks really don't fit in the aux the way it is currently defined. For instance, aux(n).name [Type: string] This is string describing the nth auxiliary data timecourse.

This really sets up aux to be auxiliary time course recordings.

Landmarks will always be related to the head anatomy and the placement of optodes and/or electrodes and/or point clouds to get head geometry. SD seems like the best place for this.

fangq commented 5 years ago

ok, I will move those to the sd then. will create another pull request.

fangq commented 5 years ago

on another thought, if I change aux(n).landmark to aux.landmark (that's what I actually meant) - would that address your concerns? or you still like sd.landmark better?

jayd1860 commented 5 years ago

Hi Qianqian,

I'm still not completely clear on this issue. As you say, digitized data may be independent of sources and detectors, yes; but I do see how digitized points can be used to position a probe, hence it's connection to the sd field. What I'm wondering is what is it's connection to aux? Why store it in that particular structure?

Or perhaps, if we do treat it as independent data that has a variety of uses, then maybe we need a new SNIRF variable called digPts, just like in AtlasViewer?

Jay

fangq commented 5 years ago

fine, I moved those to sd, also changed landmarkName to landmarkLabels to look similar to srcLabels and detLabels.