Skip to content
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

BUG: Somato BIDS dataset derivatives need to be updated #6770

Closed
larsoner opened this issue Sep 15, 2019 · 10 comments · Fixed by #7387
Closed

BUG: Somato BIDS dataset derivatives need to be updated #6770

larsoner opened this issue Sep 15, 2019 · 10 comments · Fixed by #7387

Comments

@larsoner
Copy link
Member

Turns out there was a bug with the conversion in #6414 conversion. The subject used to be somato but now it's 01 and the -src.fif was never updated. We thus end up with:

>>> fwd['src'][0]['subject_his_id']
'somato'

which is wrong (and is now arguably correctly treated as an error downstream later by MNE). I have a quick fix for this but it would be good @jasmainak @sappelhoff if someone could re-create the src, fwd, and (if applicable) inv for this subject because they will all have the old, incorrect subject_his_id value.

@larsoner larsoner modified the milestones: 0.20, 0.19 Sep 15, 2019
@jasmainak jasmainak assigned jasmainak and unassigned jasmainak Sep 17, 2019
@jasmainak
Copy link
Member

@larsoner how urgent is this? can this wait till the sprint? @sappelhoff any chance you can fix it in the coming few days?

@sappelhoff
Copy link
Member

Unfortunately I am completely swamped and won't be able to submit a fix for this :(

@larsoner
Copy link
Member Author

It can wait until 0.20, I pushed a workaround in #6771

@larsoner larsoner modified the milestones: 0.19, 0.20 Sep 17, 2019
@jasmainak jasmainak added this to To do in woodshole coding sprint (2019) via automation Sep 17, 2019
@wmvanvliet
Copy link
Contributor

Also, dataset_description.json needs to be updated with a new link to relevant docs:

http://martinos.org/mne/stable/manual/datasets_index.html?#somatosensory ->
https://mne.tools/stable/overview/datasets_index.html#somatosensory

@larsoner
Copy link
Member Author

larsoner commented Mar 4, 2020

@sappelhoff @jasmainak any chance to get to this in the next week or two to make it into 0.20?

@sappelhoff
Copy link
Member

This is the conversion script.

If I understand you correctly, the error is with these lines:

https://github.com/mne-tools/mne-scripts/blob/9e4c77b724bb04a6d20e93720b5dd6cf110ff8eb/bids_conversion/convert_somato_data.py#L227-L231

where we simply rename the existing fwd file, instead of recomputing it. As far as I see it, there are no src or inv files.

Could a fix be as simple as loading the fwd, changing subject_his_id, and saving under a new name?

@agramfort
Copy link
Member

agramfort commented Mar 4, 2020 via email

@larsoner
Copy link
Member Author

larsoner commented Mar 4, 2020

Could a fix be as simple as loading the fwd, changing subject_his_id, and saving under a new name?

I would just overwrite the old one with the correct one

@sappelhoff
Copy link
Member

@larsoner to create a new (correct) one, I assume that I would use mne.make_forward_solution

What should I supply for the src and bem parameters?

These are all the files I have (printed up to depth 3):

|MNE-somato-data/
|--- subjects/
|------ somato/
|--------- scripts/
|--------- stats/
|--------- label/
|--------- touch/
|--------- surf/
|--------- mri/
|--------- trash/
|--------- tmp/
|--------- src/
|--------- bem/
|--- MEG/
|------ somato/
|--------- sef_raw_sss-trans.fif
|--------- somato-meg-oct-6-fwd.fif
|--------- sef_raw_sss.fif

@larsoner
Copy link
Member Author

larsoner commented Mar 4, 2020

No I would not create a new one, just use read_forward_solution, modify s['subject_his_id'] for each s in fwd['src'], then write_forward_solution. It ensures that everything stays the same except the subject_his_id

woodshole coding sprint (2019) automation moved this from To do to Done Mar 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants