BEP029 Motion

[JuliusWelzel]

PR to get things started with the integration of BEP029 motion ToDos:

1. Be ready for community review
2. get Macros working

[JuliusWelzel]

I think the extra file for the BEP is working fine however, some checks still do not pass. Maybe @sappelhoff can help us out here? Thanks :)

[sappelhoff]

@JuliusWelzel you can rebase your branch on master, or (easier) merge the commits from master into your branch. You are a few commits behind (6 commits, to be exact, see screenshot), and the issues of "linkchecker" have been fixed upstream :-)

PS: Let me know if you need help with that - I don't know how advanced your git knowledge is

[JuliusWelzel]

Worked, thank you @sappelhoff

[sappelhoff]

(NOTE: I'll cross-post this message across several BEP threads)

Hi there, just a quick notification that we have just merged https://github.com/bids-standard/bids-specification/pull/918 and it may be interesting to look at the implications for this BEP.

We are introducing "BIDS URIs", which unify the way we refer to and point to files in BIDS datasets (as opposed to "dataset-relative" or "subject-relative" or "file-relative" links).

If the diff and discussion in the PR is unclear, you can also read the rendered version: https://bids-specification.readthedocs.io/en/latest/02-common-principles.html#bids-uri

Perhaps there are things in the BEP that need adjusting now, but perhaps also not -- in any case it's good to be aware of this new feature!

Let me know if there are any questions, comments, or concerns.

[rwblair]

I have started writing the schema objects and rules for bep 029 in a separate branch. https://github.com/rwblair/bids-specification/tree/schema/bep029

Wanted to post this here to prevent duplication of effort. Haven't even linted the yaml I've written so far so may not be parseable as is.

[codecov[bot]]

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.12 :warning:

Comparison is base (8f21b45) 88.01% compared to head (f613ba7) 87.89%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #981      +/-   ##
==========================================
- Coverage   88.01%   87.89%   -0.12%     
==========================================
  Files          14       14              
  Lines        1268     1272       +4     
==========================================
+ Hits         1116     1118       +2     
- Misses        152      154       +2     

Impacted Files	Coverage Δ
tools/schemacode/bidsschematools/render/text.py	96.53% <66.66%> (-0.95%)	:arrow_down:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[JuliusWelzel]

Quick questions about the ordering of entities in the filenames in this BEP. We are currently unsure what the best position for the _tracksys- key-value pair in the file name is. Is it correct, that this is a matter of the schema specification (https://bids-specification.readthedocs.io/en/stable/schema/index.html#ordering-rules) or are there more general rules about this? @sappelhoff @rwblair

[sappelhoff]

There are some more rules in this file: https://github.com/bids-standard/bids-specification/blob/d0d04a1deb7adcd2552f19ff9539cd11046a02a2/src/schema/rules/entities.yaml#L1-L3

We are currently unsure what the best position for the _tracksys- key-value pair in the file name is

yeah, good question. In case of doubt, I'd put it at the end.

[rwblair]

Technically does not matter. Main expectation is that subject and session will always be first and derivative entities tend towards the end.

Might be nice to have concrete rules that guide the ordering, but given how long anarchy has reigned that ship may have sailed. Also seems like the type of thing that could be the topic of endless bike shedding.

[JuliusWelzel]

Thanks for the quick feedback, I anticipated it to be not really relevant and guess it is dealt with in the schema automatically anyways.

[JuliusWelzel]

@tsalo I started working on the schema for the MOTION-BEP. I included a first MACRO in line 13. Can you please let me know if this goes in the right direction. I would propose to have the schema relevant changes included in this PR.

Best, Julius

[JuliusWelzel]

I added the schema for motion, unfortunately I cannot render it locally due to a filepath error

UserWarning: File 'C:\\Users\\User\\Desktop\\kiel\\bids-specification\\tools\\schemacode\\bidsschematools/data/schema/SCHEMA_VERSION' cannot be found
        warnings.warn(f"File {path!r} cannot be found")).

I would be happy for some helps with the testing of the schema :)

[Remi-Gau]

I added the schema for motion, unfortunately I cannot render it locally due to a filepath error

UserWarning: File 'C:\\Users\\User\\Desktop\\kiel\\bids-specification\\tools\\schemacode\\bidsschematools/data/schema/SCHEMA_VERSION' cannot be found
        warnings.warn(f"File {path!r} cannot be found")).

I would be happy for some helps with the testing of the schema :)

@JuliusWelzel

The motion page should now render online: https://bids-specification--981.org.readthedocs.build/en/981/modality-specific-files/motion.html

I personally tend to rely on the CI build of the doc and rarely try to serve the doc locally. I just make small commits that I push regularly and if things start failing I can usually identify where things went "boing".

I had to comment some of the things you had put in yml because it was causing issues either with the schema validation or the actual build of the doc.

Let me know if something is unclear.

[Remi-Gau]

oh and @JuliusWelzel, remember to pull my commits before pushing new things. 😉

[JuliusWelzel]

@Remi-Gau I updated the schema object, can you please have a look if everything is correct? I also pushed to use CI build to catch errors

[JuliusWelzel]

I would also remove the tables and only start relying on the macros if everyone is fine with that. :D

[Remi-Gau]

It looks fine to CI (except for a few trailing whitespaces that we can ignore for now) and therefore looks fine to me.

Definitely start implementing macros. In your own time.

[sappelhoff]

to be merged before continuing work here:

• [x] https://github.com/JuliusWelzel/bids-specification/pull/5

(and git pull locally, before continuing work after that PR has been merged)

[JuliusWelzel]

Dear all, Sein and I went over all your helpful suggestions and comments and incorporated them to make the PR community release ready.

I still have some minor questions which should not block moving forward:

1. Some fields (for example SubjectArtifactDescription are used in this BEP. The description from the schema can be misleading sometimes, adding a description_addendum only helps in some cases. Is there a way to overwrite the description when rendering?
2. The description in the *_channels.tsv provide a MUST in which order they have to appear. Is this something that every other BIDS modality should follow if a certain column is present in a *_channel.tsv file?
3. The *_motion.json has a field for TrackingSystemName which should match the key tracksys-<label> in the filename? Is this necessary as such?
4. The schema is still missing checks for this BEP. How urgent are these?

If there are no other remarks, we are happy to move into community review.

Best, Sein & Julius

[sappelhoff]

Some fields (for example SubjectArtifactDescription are used in this BEP. The description from the schema can be misleading sometimes, adding a description_addendum only helps in some cases. Is there a way to overwrite the description when rendering?

Can you please elaborate what the concrete problem is (or what they are in case there are several)?

For the actual question, perhaps @tsalo or @effigies can help.

---

The description in the *_channels.tsv provide a MUST in which order they have to appear. Is this something that every other BIDS modality should follow if a certain column is present in a *_channel.tsv file?

Can you please clarify what you mean by that? Maybe using an example?

---

The *_motion.json has a field for TrackingSystemName which should match the key tracksys-<label> in the filename? Is this necessary as such?

What exactly is the question? Is it whether the TrackingSystemName metadata is needed when it's in the tracksys entity? Or is it whether there SHOULD be a correspondence?

My take is that:

1. that metadata is needed, and should be REQUIRED
2. a correspondence between the metadata and the entity MUST be enforced (checked)

... unless there are situations where this doesn't make sense ... are there?

---

The schema is still missing checks for this BEP. How urgent are these?

IMHO we can implement these in parallel to the community review and release process.

[effigies]

Agreed overall that questions 1 and 2 are unclear. Links to sections in the doc or code, as well as a description of what is wrong, would be helpful.

The *_motion.json has a field for TrackingSystemName which should match the key tracksys-<label> in the filename? Is this necessary as such?

What exactly is the question? Is it whether the TrackingSystemName metadata is needed when it's in the tracksys entity? Or is it whether there SHOULD be a correspondence?

My take is that:

1. that metadata is needed, and should be REQUIRED

2. a correspondence between the metadata and the entity MUST be enforced (checked)

... unless there are situations where this doesn't make sense ... are there?

As it currently reads, it seems that it's just two names for identical strings. If it's a required entity, then why duplicate it in the metadata? My suggestion would be that the entity is required and the metadata be OPTIONAL if someone wants to record a more human-readable name.

I would not enforce anything on the metadata. If the entity and metadata field are unrestricted but must be in 1-1 correspondence throughout the dataset, it will be painful to enforce this, as it requires a global store for each pair of things that are so matched. If the correspondence is something like "the entity label must be some simple transformation of the metadata key", that is plausible to enforce on individual files, though probably ugly and the value is unclear.

The schema is still missing checks for this BEP. How urgent are these?

IMHO we can implement these in parallel to the community review and release process.

Agreed.

[JuliusWelzel]

Some fields (for example SubjectArtifactDescription are used in this BEP. The description from the schema can be misleading sometimes, adding a description_addendum only helps in some cases. Is there a way to overwrite the description when rendering?

Can you please elaborate what the concrete problem is (or what they are in case there are several)?

For the actual question, perhaps @tsalo or @effigies can help.

Sorry for not being very specific. We want to use the field SubjectArtefactDescription in our BEP. The description of this field from the current BIDS release (1.8.0) reads as follows: "Freeform description of the observed subject artefact and its possible cause (for example, "Vagus Nerve Stimulator", "non-removable implant"). If this field is set to "n/a", it will be interpreted as absence of major source of artifacts except cardiac and blinks."

For motion data the examples for artefacts (Vagus Nerve Stimulation and "non removable implant") do not make a lot of sense (Walking aid would be a more useful example here). Is there a way to provide a description text in the metadata schema object which overwrites the original description only for the rendering of the motion specifications?

The description in the *_channels.tsv provide a MUST in which order they have to appear. Is this something that every other BIDS modality should follow if a certain column is present in a *_channel.tsv file?

Can you please clarify what you mean by that? Maybe using an example?

Sorry, this is resolved now.

The schema is still missing checks for this BEP. How urgent are these?

IMHO we can implement these in parallel to the community review and release process.

Thank you for the information.

[effigies]

Is there a way to provide a description text in the metadata schema object which overwrites the original description only for the rendering of the motion specifications?

No. What we should do is change the base definition to be applicable to all use cases, and then add description_addendums to each place it's used such that it needs additional clarification.

[JuliusWelzel]

Agreed overall that questions 1 and 2 are unclear. Links to sections in the doc or code, as well as a description of what is wrong, would be helpful.

The *_motion.json has a field for TrackingSystemName which should match the key tracksys-<label> in the filename? Is this necessary as such?

What exactly is the question? Is it whether the TrackingSystemName metadata is needed when it's in the tracksys entity? Or is it whether there SHOULD be a correspondence? My take is that:

1. that metadata is needed, and should be REQUIRED

2. a correspondence between the metadata and the entity MUST be enforced (checked)

... unless there are situations where this doesn't make sense ... are there?

As it currently reads, it seems that it's just two names for identical strings. If it's a required entity, then why duplicate it in the metadata? My suggestion would be that the entity is required and the metadata be OPTIONAL if someone wants to record a more human-readable name.

I would not enforce anything on the metadata. If the entity and metadata field are unrestricted but must be in 1-1 correspondence throughout the dataset, it will be painful to enforce this, as it requires a global store for each pair of things that are so matched. If the correspondence is something like "the entity label must be some simple transformation of the metadata key", that is plausible to enforce on individual files, though probably ugly and the value is unclear.

Currently the field is REQUIRED (https://github.com/JuliusWelzel/bids-specification/blob/a1bf35feb5c5971d4b7407dd2d6e6bc54a16bd5b/src/schema/rules/sidecars/motion.yaml#L56) in the *_motion.json. It is explained how to use it here (https://github.com/JuliusWelzel/bids-specification/blob/a1bf35feb5c5971d4b7407dd2d6e6bc54a16bd5b/src/modality-specific-files/motion.md?plain=1#L40). Should we continue with the field being REQUIRED or change it to be OPTIONAL?

[effigies]

Should we continue with the field being REQUIRED or change it to be OPTIONAL?

Why was it required in the first place? I don't know if we have this written anywhere, but here's what's in my head when making these decisions:

• REQUIRED: Data can't be interpreted without this information (or the ambiguity is unacceptably high)
• RECOMMENDED: Interpretation/utility would be dramatically improved with this information
• OPTIONAL: People/tools might find it useful to have this information

Every required field is a barrier to somebody using the standard, and you're basically stating "Data without this field might as well not be shared".

[sappelhoff]

As it currently reads, it seems that it's just two names for identical strings. If it's a required entity, then why duplicate it in the metadata?

My initial thought was so that one wouldn't have to parse the file name but could simply load the JSON metadata. But that's a very weak argument, as it's easy to implement and virtually all BIDS tools already do that.

I would not enforce anything on the metadata. If the entity and metadata field are unrestricted but must be in 1-1 correspondence throughout the dataset, it will be painful to enforce this, as it requires a global store for each pair of things that are so matched.

okay, you convinced me 👍

My suggestion would be that the entity is required and the metadata be OPTIONAL if someone wants to record a more human-readable name.

I like this suggestion as long as it's made explicit that the tracksys METADATA in JSON is an optional field to encode a more human readable name corresponding to the required tracksys ENTITY.

[sappelhoff]

open points:

• [x] moving tracksys entity up in the ordering (before run)
• [x] last point from here

[Moo-Marc]

Hello, My 2 cents: for SpatialAxes, should we use Anterior/Posterior and Superior/Inferior instead of Forward/Back and Up/Down? Cheers

[smakeig]

Do I understand correctly that this standard assumes that specifying a starting time and sampling rate is sufficient to synchronize multiple time series recording modalities? If so, I beg to differ. In our experience using LabStreamingLayer (LSL) recording of multimodal EEG brain imaging + motion capture (what we refer to as MoBI) experiments, we have seen instances in which unexpected (and typically unmonitored) behavior by recording devices have made data interpreted using these assumptions wholly useless. If, as is certainly possible, the mismatch between the quoted and actual sampling rate of some recording modality is as small as 0.001%, (e.g., 0.01 Hz for 1kHz recording) then over the course of an hour of recording, the timing mismatch, relative to other streams, will grow to 36 ms (correct?) . This is unacceptable for detailed kinematic, biomechanical, or electrophysiological studies. Still more problematic is the assumption that a device with a putative fixed sampling rate will actually deliver that - as most labs need to minimize equipment costs and the cheaper the equipment, the lower the probability (in general) that strict timing fidelity was a design objective (as opposed to expensive and precisely engineered systems such as fMRI scanners). I have had senior imaging center engineers ask me, "But which modality is the standard to which all others are referenced? (no doubt thinking of 'fMRI' or 'MEG'). In the ever-widening world of brain/body research (conducted for a palette of purposes), there is typical no such central highly-engineered (and expensive) 'central' system.

For this reason LSL saves the exact arrival times of each data sample in each data stream as an auxiliary channel, and allows these channels to be downloaded with the multimodal data (in .xdf format). To save raw multimodal data - involving motion capture or not - the time marker channels need to be saved and identified, allowing synchronizing by simple [(end-start)/srate] or more cautious/sensitive/nonstationary algorithms. And in particular allowing recovery of synchronicity in cases where simple synchronization approaches prove inadequate.

As there is opposition, in some BIDS corners at least, to recognizing LSL and .xdf as important / widely-used standards, two alternatives for storing multimodal data come to mind. First, the time marker channel for each modality can be saved in an aux folder using some 'off-standard' LSL convention. Or second, a BIDS BEP could be proposed to save only the time marker channels (however recorded) for a multimodal recording. These would then be available in a BIDS-designated fashion for multimodal data analysis. A third, also 'off-BIDS label' solution might simply add the marker time channel to its respective data matrix, though this seems inelegant as it would require any, e.g., BIDS app to identify and remove or deal separately with these channels when present.

Thoughts/comments appreciated,

Scott Makeig

[sjeung]

Hello, My 2 cents: for SpatialAxes, should we use Anterior/Posterior and Superior/Inferior instead of Forward/Back and Up/Down? Cheers

Hi @Moo-Marc, thanks for your comment. The reasoning behind the choice Forward/Backward over Anterior/Posterior is that we do not want to restrict the type of tracked objects to human body parts.

Another reason is that in some contexts the positions and orientations along the global spatial axes are more useful than the same values represented in the anatomical reference frame.

For instance, example data set "dualtask" contains positions collected from an optical mocap system and it records walking behavior of a participant up and down a line in the lab space, which is then represented as the Forward/Backward axis.