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

Feat/interpolate channels #480

Merged
merged 58 commits into from
Nov 10, 2023

Conversation

gcattan
Copy link
Contributor

@gcattan gcattan commented Sep 10, 2023

This PR modifies the match_all method inside base paradigm, which set-up the paradigm to be compatible with a list of datasets.
It is now possible to select the intersection of the channels in all datasets, or their union.
For example, if you pass two datasets:
d1 -> Cz, Fz
d2 -> Cz, C4,
then the channels attribute inside the paradigm can contain either: Cz or {Cz, Fz, C4}.

In the case you selected the union of all channels, then missing channels in dataset d1 or d2 will be interpolated using mne.
This interpolation is done directly in the RawToEpochs transformer.

@bruAristimunha FYI

@gcattan
Copy link
Contributor Author

gcattan commented Oct 11, 2023

@bruAristimunha @sylvchev FYI

# Trick: mark these channels as bad
raw.info["bads"].extend(missing_channels)
# ...and use mne bad channel interpolation to generate the value of the missing channels
raw.interpolate_bads(origin=(0, 0, 0.04))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @gcattan , I really like your idea about channel interpolation to match electrodes of two datasets.
Is there a reason why in interpolate_bads you have selected origin (0, 0, 0.04) ? Option "auto" will fit a sphere to headshape based on the existing electrodes and determine its radius and origin.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! I am not sure it will work if the montage is disabled. But let me try.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Sara04 I added a try-catch, so we first try with the 'auto' parameter, and default to (0, 0, 0.04) in case of ValueError. This happens, for example, when there is no montage information. The braindecode pipeline fails, but I don't think the issue originates from this PR.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @gcattan ! Yes, it doesn't seem that it fails due to this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All checks passed. @Sara04 FYI

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, @gcattan !

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. Is it ok to merge this PR then?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me it is OK. @sylvchev , @bruAristimunha do you have some other comments?

@gcattan gcattan reopened this Oct 21, 2023
@bruAristimunha bruAristimunha enabled auto-merge (squash) November 10, 2023 16:25
@bruAristimunha
Copy link
Collaborator

Thank you @gcattan and @Sara04

@bruAristimunha bruAristimunha enabled auto-merge (squash) November 10, 2023 16:25
auto-merge was automatically disabled November 10, 2023 16:33

Head branch was pushed to by a user without write access

@gcattan
Copy link
Contributor Author

gcattan commented Nov 10, 2023

@bruAristimunha @Sara04 whats_new completed.

@bruAristimunha bruAristimunha merged commit e7f465c into NeuroTechX:develop Nov 10, 2023
10 checks passed
@bruAristimunha
Copy link
Collaborator

Done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants