-
Notifications
You must be signed in to change notification settings - Fork 189
[ENH] BEP042 - extension for electromyography (EMG) #1998
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
cc @agramfort |
0d01783 to
8902705
Compare
|
Hi, @neuromechanist pointed me to this PR and I would like to share some thoughts. This seems to be pretty advanced in terms of sensor placement description which was not very well defined in the motion BEP :)
|
|
Hi @sjeung, thanks for the feedback / ideas.
done in e84cadc
Those are skeletal landmarks, not muscles. But we've reworked EMGPlacementScheme to be an enum now, so that example will need to change anyway.
This was the intent, perhaps it's just not worded clearly enough? Suggestions for clarification are welcome.
For EEG, I think abrasive gel isn't used because of possible damage to hair. According to @neuromechanist it would be odd to use a different skin prep for different EMG sites in the same session, so we'll probably leave this as as-is. |
|
I think this BEP is ready for a thorough review by the team: @robertoostenveld @larsoner @neuromechanist @arnodelorme @JuliusWelzel @tjeerdboonstra cc @agramfort |
|
The document looks comprehensive https://bids-specification--1998.org.readthedocs.build/en/1998/modality-specific-files/electromyography.html. A few comments:
|
In the majority of cases yes. But not necessarily, if there are e.g. some grid devices and some bipolar devices at different spots on the body, recording into separate amplifiers / data files but acquired simultaneously. These would get different values for the
good catch. That should be
I went back and forth on that question.
I think this is actually correct as-is. Other coordinate systems are allowed for other modalities, but we're making the assumption that things like |
|
Hello, thanks very much for the great progress! I have some remarks listed below:
In EMG BIDS the Also the
|
That is an attractive idea. You are right that we chose
it is explained at the end of the initial "EMG Data" section (just before the "Terminology: electrodes vs channels" subsection). It comes up again when discussing coordsystems, and then again when discussing photos. I couldn't see a good way to avoid talking about it in multiple places. In light of that, do you still think it needs to move / change?
Are you specifically asking to add the "sub-millisecond precision" bit? (if so, no objection). If not, can you clarify what you think is lacking here?
what is MUST, SHOULD, or MAY is open to discussion. There are also likely some more rules to be added, e.g., to make some optional fields required depending on the values in other fields. Regarding specifically the Polhemus case, I would agree that digitized locations MUST include
thanks, fixed. It was asking for
thanks, fixed.
EDF/BDF necessarily have channel names in the file (which I think is what you mean by "headers" right?). There are also guidelines on what the format of such channel names should look like (modality-space-identifier, i.e.,
This was originally copy-pasted from EEG, then pruned. I agree it needs refinement... I had a code comment in there for a while saying as much, until I realized almost nobody was reading the source :) For now I'll remove PUPIL, EYEGAZE, ADC, DAC, and OTHER, and add POS. |
|
Thanks, @JuliusWelzel, very insightful comments,
IMHO,
And the description of how to use it:
|
Good point, I think it can stay as it is. Maybe it is worth adding a detailed explanation for the reasoning in the paper.
Yes, sub-millisecond presicion is imo worth mentioning as EMG usually has a high srate. This time resolution is important for good syncronization with other modalities.
I am not aware of any case where it is not provided in x,y,z.
True, sorry. But maybe it can be pointed out, that the names in the 'channels.tsv' MUST match the names in the BDE/EDF file? |
Agreed, maybe a PR can be opened to extend the definition for the
Good idea, I would be in favor off adding LATENCY to the channel types. The scans.tsv file will also be replace with a recordings.tsv file in BIDS 2.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made it through once and made suggestions. The overall specifications look great. I look forward to the community comments.
|
ping @robertoostenveld and @tjeerdboonstra. I think we're about ready to open this up to public comment; do you want a chance to go through it again first? |
|
idea. So if to parallel exactly, should get |
Yes! As far as I understand how
This resulted in a REQUIRED
As for the terminology, adopting |
agreed, |
|
@effigies re: our conversation earlier about needing to back out earlier changes: I think we want to revert c6d4388 and 8b0393a in this PR, right? IIUC we won't need it; most of what we need will be in the new |
|
@effigies and @neuromechanist I think now it's time to squash all commits here, per review policy? For me locally, all the datasets at neuromechanist/bids-examples#3 pass validation when run against bids-standard/bids-validator#268 and the CIs here are happy too |
Co-authored-by: Seyed (Yahya) Shirazi <[email protected]> Co-authored-by: Jörn M. Horschig <[email protected]> Co-authored-by: Chris Markiewicz <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not had time to dig into lots of details here unfortunately but reading the preview at https://bids-specification--1998.org.readthedocs.build/en/1998/modality-specific-files/electromyography.html you have my +1. It covers the use cases I have top of mind. thanks @drammock @neuromechanist for pushing this and all reviewers who helped bring this proposal to this level 👏
|
Nice job! Here are some minor comments/suggestions:
|
|
+1 on my side as well. |
|
Please review the EMG-BIDS proposal, an initiative aimed at integrating EMG data sharing within the BIDS ecosystem. Your review will contribute to the development of specifications that strike a balance between thoroughness and clarity and ease-of-use. To conduct a code review, you can examine the files modified from the tabs above. Alternatively, you can review the HTML preview of the proposed specifications and/or the examples. In the end, please provide your comments on #2219, or upvote and approve the proposed specifications. |
|
Important The final review of the BEP042 is up, Please consider voting and proving your feedback: #2219 @agramfort, @VisLab, @arnodelorme, @larsoner, thanks much for your feedback and review here. Please consider voting on the first post on the Discussion Thread 🙌 |
|
@VisLab (re: #1998 (comment))
Thank you for the suggestion, we're still discussing this.
fixed in 26346de
I actually cannot find cases where this is inconsistent and also under our control. non-code typeface occurs in right-sidebar table-of-contents (which is stripped of formatting) and in the titles of code blocks for example files (a context which doesn't allow formatting). For |
|
@klotz-t (re: #2219 (comment))
added
added additional explanation and cross-references in e009b96. LMK if this clears up your questions, or needs additional tweaks. |
|
@JuliusWelzel (re: #2219 (comment))
I've harmonized these in 4e9adcd
Thanks for this suggestion. I've added a modified version of your suggested text in afaa9e1, LMK if you think it needs any further edits. |
|
@JuliusWelzel (re: #2219 (comment)):
We added a sample dataset based on ds0003739, by separating the motion and EMG data from the EEG data files. It is available as of bids-standard/bids-examples@1758df9 |
UPDATE: 09/29/2025
Important
The final review of the BEP042 is up (9/29/2025 to 10/10/2025), Please consider voting and providing your feedback: #2219
This adds support for Electromyography (EMG) datasets.
CIs are not expected to pass yet.cc @neuromechanist @jwelzel @larsoner @arnodelorme @robertoostenveld feel free to push directly to this branch, I'll add you as repo collaborators on my fork.
We meet regularly to discuss this BEPNext meeting: 18 Dec 2024 on https://ucsd.zoom.us/j/96433382377Communication channel on github repo / matrix / slack / discord : #1371Note
See the HTML version of the proposped specifications at this readthedocs page
closes #1371