[WIP][ENH] BEP030: Functional Near-Infrared Spectroscopy

[rob-luke]

BEP030: Functional Near-Infrared Spectroscopy This PR adds functional near-infrared spectroscopy to BIDS. Leads: @rob-luke & @lpollonini

• validator: https://github.com/bids-standard/bids-validator/pull/952
• example: https://github.com/bids-standard/bids-examples/pull/305
• rendered specification draft: https://bids-specification.readthedocs.io/en/bep030/04-modality-specific-files/11-functional-near-infrared-spectroscopy.html

Progress:

• [x] Community reviewed proposal (google docs)
• [x] Transition google doc to markdown (this PR). View current render here.
• [x] Fix language in document. It currently reads as if it was written by many people (as it was), and needs to be reworked to fix sentence structure and grammar.
• [x] Extend validator to support this BEP (WIP: GitHub PR link)
• [x] Generate open datasets from several labs and ensure they are compliant with specification (e.g. dataset from rob-luke, dataset from robertoostenveld, these are listed in the manuscript)
• [x] Add example dataset to bids-examples (dataset 1, dataset 2)
• [ ] Preprint of the new extension (manuscript hs been reviewed by half the authors, about to be circulated to remaining authors).
• [ ] Press release

NOTE: At all stages in this process we will actively advertise and invite feedback on the proposal. We invite all feedback no matter how minor or major. And we will advertise the draft website to the community for review once it is rendering correctly.

closes #438

---

Scope of proposal:

This specification applies to the storage of raw fNIRS data and metadata, and does not address data processing or the storage of processed data. This specification aims to deal with data from well established commercial systems, not experimental or DIY systems, although everyone is encouraged to adhere to BIDS-fNIRS to facilitate future data sharing. The specification currently only addresses continuous wave (CW) fNIRS devices, but consideration is taken not to preclude extensions to TD and FD instruments in the future upon contributions from experts in these domains.

Considerations when reviewing this BEP:

• It was decided to only include continuous wave (CW) fNIRS in this BEP. However, we have taken effort not to preclude expansion to other techniques such as time or frequency domain fNIRS. We ask that TD and FD experts still review this proposal to ensure nothing in the specification precludes future expansion to other fNIRS types.
• This BEP does not specify details about hyper scanning, which is popular in the fNIRS community. This was decided as hyper scanning is not specific to fNIRS and is also performed using other modalities. Thus, hyper scanning should be added to BIDS in a modality agnostic manner. Hyperscanning is an open issue in BIDS (https://github.com/bids-standard/bids-specification/issues/402).
• A conscious decision was made not to specify what should be considered a short channel. The specification contains fields to store the number of short channels, as this will be important for searchability. But the definition of what exactly constitutes a short channel (is it less than 8mm, or 10mm, etc) is left to the end user and software packages.

---

To provide feedback:

1. Select Files Changed in the tab list at the top of this text box.
2. Press the Load Diff button for the src/04-modality-specific-files/10-functional-near-infrared-spectroscopy.md file
3. Hover your mouse over any line that you wish to comment on, this should make a blue cross appear to the right of the line.
4. Press the blue cross and provide enter your comment

[rob-luke]

@sappelhoff do you know why the types aren't rendering as hyperlinks in my tables? I think I copied the format from the other BEPs. Thanks

[sappelhoff]

@sappelhoff do you know why the types aren't rendering as hyperlinks in my tables? I think I copied the format from the other BEPs. Thanks

you probably forgot to copy over the link targets, the [this is a link][] syntax only works if you declare [this is a link]: https://www.google.com somewhere in the document (that latter declaration will be invisible in the rendering).

see: https://github.com/bids-standard/bids-specification/blame/master/src/04-modality-specific-files/03-electroencephalography.md#L433-L447

[rob-luke]

Thanks so much @sappelhoff, that makes total sense now. Appreciate it.

[rob-luke]

Thanks for the information @sappelhoff. Congratulations on the big PR merge, its great. I will adjust this PR to use the new scheme when I have some time in next weeks.

[rob-luke]

Thank you for the review @guiomar.

[rob-luke]

@sappelhoff is there a way to pass additional text to a schema? for example I would prefer this text for CapManufacturer

Name of the cap manufacturer (for example, "Artinis") If a custom-made cap is used then the string “custom” should be used. If no cap was used, such as with optodes that are directly taped to the scalp, then the string “none” should be used and NIRSPlacementScheme field may be used to specify the optode placement scheme.

but currently the schema provides:

Name of the cap manufacturer (for example, "EasyCap").

If not, should I create a new schema yaml called "CapManufacturerNIRS.yaml"? or revert to the hard coded table?

[sappelhoff]

is there a way to pass additional text to a schema?

Take a look at the following line: https://github.com/bids-standard/bids-specification/blame/master/src/04-modality-specific-files/03-electroencephalography.md#L119

That's an example of how the schema "base definition" can be extended/tweaked slightly to better fit the context. Would that work for you? Else, let's hear @tsalo's opinion.

[rob-luke]

Hi everyone,

Just a quick update on the progress here from @lpollonini and I.

• The file format that this BEP imposes has been released as version 1.0 (https://github.com/fNIRS/snirf/pull/82), that clears the last thing blocking this BEP.
• We are holding off on submission of the proposal until the @dboas team provide feedback on their experience implementing fNIRS-BIDS in https://github.com/BUNPC/Homer3 (the most prominent of fNIRS software packages). We are in regular contact and no issues have arisen so far. Should no issues arise by the end of November we will push forward with the BEP.
• We have a draft of the paper with the BEP leads. This will be circulated amongst contributors in the coming weeks.
• Two weeks before planned submission of the BEP we will rebase and correct the code underlying this PR (plus make the fix suggested by @helenacockx above and anything else that arises in the meantime). Same with the validator.
• We have several example datasets published on OSF for users to download. I will add a data-zeroed example to bids-examples in the coming weeks.

In summary, we are looking good for a submission at the latest mid December, earlier if we get the green light from the Homer developers.

[rob-luke]

Thanks @Remi-Gau! The suggestions look great. I will get to making the changes in the next few weeks, just waiting on feedback from another lab too.

[rob-luke]

Here is a brief status update following from https://github.com/bids-standard/bids-specification/pull/802#issuecomment-940562988.

• Several issues/implementation inconsistencies were found with the SNIRF file format which was being conflated with problems with the BIDS proposal. The SNIRF file format is the only format supported in this BEP. So while it isn't strictly part of this extension proposal, it was essential to address. This has taken engineering effort away from this BEP. However, there is now an official SNIRF validator (https://github.com/BUNPC/pysnirf2) so this issue should be largely resolved and attention can return to finalising this BEP PR.
• An initial draft of the manuscript has been circulated around half the co-authors. Most feedback has been incorporated. Before the draft will be shared with the remaining co-authors we are waiting on an additional two example datasets.
• The Homer software team has spent considerable time planning and implementing BIDS support, no significant issues have been raised.
• A data-zeroed dataset has been submitted as a PR to the bids-examples repository which will allow testing of the validator. More work is required here.
• Suggestion were made on this PR by Helena, Robert, and Remi that need to be implemented.

So things are moving a bit slower than planned, but we are slowly moving forward.

[codecov[bot]]

Codecov Report

Base: 88.57% // Head: 88.57% // No change to project coverage :thumbsup:

Coverage data is based on head (69a2db3) compared to base (959b646). Patch has no changes to coverable lines.

:exclamation: Current head 69a2db3 differs from pull request most recent head ca9550a. Consider uploading reports for the commit ca9550a to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #802   +/-   ##
=======================================
  Coverage   88.57%   88.57%           
=======================================
  Files           6        6           
  Lines        1042     1042           
=======================================
  Hits          923      923           
  Misses        119      119           

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 at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[claytonjschneider]

Hi all I am a neuroimaging engineer with expertise in Python and software engineering in science, looking to help move this particular aspect of the BIDS standard along. It is a goal of mine to contribute to BIDS before grad school. If there are any changes I can assist with, please let me know! It will be very beneficial for our lab to have fNIRS / BIDS compatibility, and I look forward to developing tools that take advantage of this consensus.

[rob-luke]

Hi @claytonjschneider, thanks for the offer of help. I have recently reformatted the specification document; it would be great to have a fresh set of eyes read through it and suggest any changes (you can use the GitHub review feature). And let us know if you can foresee any issues formatting your dataset using this specification. You mention that you have engineering experience. We need to add some tests to the validator code (see https://github.com/bids-standard/bids-validator/pull/952#issuecomment-1025527076). You could give that a crack too if you have some time (again, either use the github review feature or open a PR on the PR).

[rob-luke]

For all NIRS fields that do not yet have a corresponding metadata YAML file (see: https://github.com/bids-standard/bids-specification/tree/master/src/schema/metadata) you'll have to create such a YAML file.

Done. But I have two issues I can't figure out how to fix @sappelhoff, can you suggest how I can fix these...

[robertoostenveld]

can we get this rendered as bep-030 on readthedocs (with the version selector in the lower right corner)? There is a bep-009 and bep-028 there. I also propose to remove the wip-derivatives

[sappelhoff]

can we get this rendered as bep-030 on readthedocs (with the version selector in the lower right corner)? There is a bep-009 and bep-028 there. I also propose to remove the wip-derivatives

done, I also removed wip-derivatives (cc @effigies)

[rob-luke]

@sappelhoff I am unable to understand why the schemacode tests are failing. I have not edited any of the schemacode files, and I can't decipher the error. Could you please point me to the correct person to ask about this?

[sappelhoff]

Ah sorry, just saw it now. I'll look into it, but @tsalo and @effigies are well versed with this.

[sappelhoff]

also, merging master into this branch seems to have broken the build.

[effigies]

Also https://github.com/bids-standard/bids-specification/pull/1107 is going to cause issues in this PR. Happy to apply the fix once it's merged.

Also can update your nirs.yaml with the above-mentioned changes directly if you prefer that to me making suggestions and you applying them.

[sappelhoff]

Thanks @effigies! I think @rob-luke will be happy for each of your commits (unless the situation has changed and he suddenly has a lot of time on his hands :wink:)

[rob-luke]

I think @rob-luke will be happy for each of your commits

@sappelhoff is correct, any direct commits for improvements are greatly appreciated @effigies I think I managed to fix the current issue and get those green ticks. But any other changes are appreciated too :)

[rob-luke]

thanks for the changes

I still have a few more to address, but I am working through your great comments. Thanks for your attention to detail. I will finish addressing your comments in the coming two days.

What do you all think?

Great plan

[sappelhoff]

@rob-luke @lpollonini we have scheduled the community review period for the NIRS BEP to be for 14 days from 2022-09-01 until 2022-09-14.

Given that no major problems are still present at the end of the review period, we will:

1. merge NIRS
   • spec: https://github.com/bids-standard/bids-specification/pull/802
   • validator: https://github.com/bids-standard/bids-validator/pull/952
   • examples:
     • 1. https://github.com/bids-standard/bids-examples/pull/305
     • 2. https://github.com/bids-standard/bids-examples/pull/323
2. continue directly with a 5-business-day long freeze of the specification
3. followed by a release of BIDS 1.8.0 :tada:

I hope this suits your schedules! You can read some reasoning in https://github.com/bids-standard/bids-specification/issues/1118#issuecomment-1183074401

The time until the community review period can now best be used to smooth everything out as much as possible (spec, validator, examples, preprint/article), so that during the review period, emerging items can be addressed quickly and efficiently and so that no new blockers arise.

Having seen the progress of this BEP and the involvement of many diverse parties, I am very confident that this will be a great success :-)

Let me know if you have any questions.

cc @bids-standard/steering @bids-standard/maintainers @bids-standard/bep_leads

[rob-luke]

we have scheduled the community review period for the NIRS BEP to be for 14 days from 2022-09-01 until 2022-09-14.

Wonderful, thank you @sappelhoff. In the meantime, we will resolve the few remaining comments and ensure that everything is ready for the review.

FYI: @dboas

[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.

[sappelhoff]

@rob-luke @lpollonini just a note that it'd be a good time to address all currently still unresolved comments on this pull request, because the community review period will start soonish (September 1), and it'd be good to have a clean slate then.

Regarding the current CI failures, @rwblair will push some commits to make the required updates, as far as I understood from the last maintainers meeting notes.

[rwblair]

I'm done with my first pass at the schema rules. I still need to update the tables to use the new style macros and confirm that they look good. Maybe more changes next week as we look at how the validator interacts with these schema updates. @effigies any comments on the checks added would be nice.

[rwblair]

to do --> nirs is currently not listed as a "datatype" under:

https://bids-specification--802.org.readthedocs.build/en/802/02-common-principles.html#definitions

Should be fixed as of 36202ca

[JuliusWelzel]

One question is: Why is ACCELChannelCount (etc.) a thing at all? It's presumably redundant with looking at the actual contents of the type column in the channels.tsv file. It's not clear what problem it's solving while it is a place for inconsistency to creep into a dataset that means we need to validate it.

The BEP029 (Motion) introduced something similar (ACCChannelCount) in 1817738f1410eabba966ef62b0e80d1c01bec76e. This was when we still optioned for the nested json structure for multiple tracking systems. This solution is now deprecated. Tbh, I also think it is kind of redundant to have the number of a channel type as an extra field and we will remove it. Additionally, we should keep namings of types consistent (we will check against BEP030 for BEP029).