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

Synchronise multiple signal #660

Merged
merged 17 commits into from
Feb 9, 2024

Conversation

Edouard2laire
Copy link
Contributor

@Edouard2laire Edouard2laire commented Nov 23, 2023

Hello,

This process aims at aligning multiple signals acquired with different devices but sharing synchronizing events (typically sent to each recording device at the time of the acquisition). This situation can arise easily when recording multimodal data such as EEG - iEEG, EEG - fMRI, fNIRS - fMRI, or EEG - fMRI.

Having the signal synchronised makes making easier to perform multimodal analysis, notably allowing the simultaneous exploration of the different signals. For example, here is a figure I recently presented at a conference, showing how we can understand better what is happening during a seizure by looking at the EEG, fNIRS, and pulse oximeter

image

What's left to do:

  • Fix the code to save continuous files as output
  • Improve file history

Future work

Here is a list of future things we might consider implementing but that is not required for this PR

  • Allowing the simultaneous visualization of the different signals from Brainstorm (opening multiple continous viewer at the same time. #656)
  • Explicitly linking the different synchronized files. This could notably be used so that if an event is added to a file, then the same event would be added to all the events the file is synchronized with.
  • Add the option to either crop the longer signal keeping only the common signal (current implementation) or pad the shorter signal with either 0 or NaN

Let me know if you want me to send you an example dataset. I can generate a minimum brainstorm database with 2 files to synchronize.

Regards,
Edouard

@Edouard2laire Edouard2laire marked this pull request as ready for review November 23, 2023 23:08
@rcassani
Copy link
Member

Let me know if you want me to send you an example dataset. I can generate a minimum brainstorm database with 2 files to synchronize.

@Edouard2laire, yes, please send me a minimal example dataset

@Edouard2laire
Copy link
Contributor Author

Edouard2laire commented Jan 31, 2024

Hi.
DId you had time to review this PR and #656 @rcassani ?

Thanks a lot,
Edouard

@rcassani
Copy link
Member

@Edouard2laire, apologies for the delay. there was a misunderstanding, I thought the PR still was on development. I will review it ASAP

@rcassani
Copy link
Member

rcassani commented Feb 2, 2024

@Edouard2laire, the main change in the process is that files are loaded one by one in memory.

Minor changes:

  • It now removes the completely out-of-range events, and fixes the partially out extended events.
  • List of synch files in History

@rcassani rcassani self-requested a review February 2, 2024 19:21
blocA = zeros(1 , length(tmp_time_a));
for i_event = 1:size(sEvtSync(iInput).times,2)
i_intra_event = panel_time('GetTimeIndices', tmp_time_a, sEvtSync(iInput).times(i_event) + [0 1]');
blocA(1,i_intra_event) = 1;
Copy link
Member

Choose a reason for hiding this comment

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

The alignment could be improved by using a Gaussian around the occurrences.

@Edouard2laire
Copy link
Contributor Author

Thanks a lot for all the changes!
I'll test it this weekend and let you know if everything works.

Edouard

@Edouard2laire
Copy link
Contributor Author

Hello.

I did few test and it seems to be working. Thanks a lot,

Edouard

@rcassani
Copy link
Member

rcassani commented Feb 9, 2024

@Edouard2laire, great!

@rcassani rcassani merged commit 34c610c into brainstorm-tools:master Feb 9, 2024
Edouard2laire added a commit to Edouard2laire/brainstorm3 that referenced this pull request Feb 9, 2024
@Edouard2laire
Copy link
Contributor Author

(Clicked wrong button — you can ignore that)

@Edouard2laire Edouard2laire deleted the sync_signals branch February 9, 2024 21:13
@rcassani
Copy link
Member

rcassani commented Feb 9, 2024

For the open tasks:

  • The feature of multiple raw time-series plots can be handled in another PR.
  • Signals are now chopped to be the same length, not sure if the alternative of zero-padding would be useful
  • Synching other events (after synching signals) for the moment can be done with Transfer events

@Edouard2laire
Copy link
Contributor Author

For the open tasks:

  • The feature of multiple raw time-series plots can be handled in another PR.

Do you want me to create the PR ? I don't really have any idea on where to start for that implementation.

  • Signals are now chopped to be the same length, not sure if the alternative of zero-padding would be useful

yes, I agree that it's not really useful. Could be implemented if someone asks. This can be the case if you still want to keep all the signals and don't have the constraint of having all the recordings all the time.

  • Synching other events (after synching signals) for the moment can be done with Transfer events

yes, that's true

@rcassani
Copy link
Member

rcassani commented Feb 9, 2024

Do you want me to create the PR ? I don't really have any idea on where to start for that implementation.

I'll start the PR, I need to check some implementation of the raw-file readers

@Edouard2laire Edouard2laire mentioned this pull request Jul 6, 2024
5 tasks
rcassani added a commit that referenced this pull request Jul 8, 2024
Issue introduced in #660 and mentioned in #719
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.

2 participants