Combine recordings

[Edouard2laire]

Hello,

This is the continuation of https://github.com/brainstorm-tools/brainstorm3/pull/660 in the attempt to enhance multimodal data analysis in Brainstorn.

Since, having multiple raw viewer recording with multiple sampling rate is not accessible (https://github.com/brainstorm-tools/brainstorm3/issues/656), i made this process to combine recording and resampling them to the highest frequency.

This allows to make such visualization directly from Brainstorm:

This assume that the signals were synchronized

Todo:

• [x] User-specified folder name
• [x] Copy channel flag
• [x] Copy video if present (not necessary actually. can be loaded from the other folder)

Question:

• [ ] Better way to merge channels? I am not sure if the way I am doing is ok; especially if I need to merge headpoints / fiducials ...

• [ ] How to deal with events ? Same thing, I am only copying events from the first file ? Should we merge them adding a sufixe to the name to identify the file ?

[Edouard2laire]

hello @rcassani

This PR is ready for review.

The only things I am not sure are: the events ( how to merge event from multiple file - esp after synchronization there will be a lot of duplicate if we just concatenate the events), and the channel flag.

Let me know if you have any questions.

I'll try to complete all the PR I have as draft soon :)

[Edouard2laire]

ok i completed my todo list.

@rcassani, let me know when you would have time to review this. Let me know if you need an example dataset

[rcassani]

@Edouard2laire, apologies for closing the PR, I was trying to close my comment —duh!

[rcassani]

@Edouard2laire, my changes are done, thanks for the patience. Please test them, then we can merge it

[Edouard2laire]

Thanks a lot. I am starting the test now.

[Edouard2laire]

it seems to be working.

However, i am getting this error a lot on the data of PA14 that I sent you:

is this something I should worry about?

It seems to be present at the end of the file:

But when I look at the nirs file at the same time it looks like this:

So the individual files are from 0 to 7056.855, 7056.9 and 7056.856 but the output is 7078.996

I am wondering why 22 sec were added at the end of the file.

[Edouard2laire]

@rcassani so it was an issue with how my file was imported. Reimported from raw file and it is now working ! Fixed a small bug for the video.

Ready to merge :)

[Edouard2laire]

Actually wait. i am trying something for the events :)

[Edouard2laire]

I made some change on the fusion of the events to avoid having multiple copy of the same events, or multiple events with the same name.

Let me know if that is ok for you.

Here is the output after the change:

[rcassani]

@Edouard2laire, thanks for noticing the part for events with same name.

Don't you think it is necessary to keep the events separated per file?

If we merge the events at combining recordings, it will not possible to split them after. Also, it is possible that events with the same name are different type (single / extended) in that case merging is not even possible. On the other hand, if we keep the events separated in the combined file, the user can easily decide to merge them. This is done with the process_evt_merge.m or in the GUI (which calls the process), and this process will handle the cases when there is same name, different types, merging if same time sample, etc.

This could be implementing by reverting to 54d1cb47 and this patch:

diff --git a/toolbox/process/functions/process_combine_recordings.m b/toolbox/process/functions/process_combine_recordings.m
index 3926c2b9..37bc1b1f 100644
--- a/toolbox/process/functions/process_combine_recordings.m
+++ b/toolbox/process/functions/process_combine_recordings.m
@@ -135,6 +135,8 @@ function OutputFiles = Run(sProcess, sInputs)
         % Concatenate events
         for iEvent = 1 : length(sMetaData(iInput).F.events)
             tmpEvent = sMetaData(iInput).F.events(iEvent);
+            % Rename event if duplicated
+            tmpEvent.label = file_unique(tmpEvent.label, {NewEvents.label});
             % Add channel info
             addedChannelNames = {NewChannelMat.Channel(sIdxChNew{iInput}).Name};
             nOccurences = size(tmpEvent.times, 2);

[Edouard2laire]

@Edouard2laire, thanks for noticing the part for events with same name.

Don't you think it is necessary to keep the events separated per file?

I don't see why it is necessary. At least I think it is important not to duplicate identical events. (since we duplicate them during the synchronization) it would lead to so many events in the combined file.

If we merge the events at combining recordings, it will not possible to split them after.

First, i dont think this is a good idea to split a combine file: we had to upsample a lot the recording. so splitting the signal again doesnt make too much sense. The combining is only here for visualization since we cant visualize multiple signal with different frequency.

But even then, it is easy to split: If an event has no channel information:it belong to all channel and can be split easily. just copy to each file. If an event has channel information, you can attribute to file based on the channel

Also, it is possible that events with the same name are different type (single / extended) in that case merging is not even possible. On the other hand, if we keep the events separated in the combined file, the user can easily decide to merge them. This is done with the process_evt_merge.m or in the GUI (which calls the process), and this process will handle the cases when there is same name, different types, merging if same time sample, etc.

I agree that we can keep the events that are not identical but have the same name and not merge them. Btw, i copied the code from the GUI and it doesnt seems it is calling the process : https://github.com/brainstorm-tools/brainstorm3/blob/master/toolbox/gui/panel_record.m#L1963-L2029

[Edouard2laire]

Would this work for you ? @rcassani

in this case, we end up with the following events:

and yes we can merge the motion latter using the GUI or process

[rcassani]

I don't see why it is necessary. At least I think it is important not to duplicate identical events. (since we duplicate them during the synchronization) it would lead to so many events in the combined file.

Makes sense for the event that was used for synchronization.

But even then, it is easy to split: If an event has no channel information:it belong to all channel and can be split easily. just copy to each file. If an event has channel information, you can attribute to file based on the channel

Yes, this is true. But there is no way to separate events based on the channels that the belong in the GUI or with a process. The user would need to program it. But by not merging (in the combining process), the user can decide to merge them, which can be done with the GUI and there is already a process.

Btw, i copied the code from the GUI and it doesnt seems it is calling the process : https://github.com/brainstorm-tools/brainstorm3/blob/master/toolbox/gui/panel_record.m#L1963-L2029

Oh! I'll add this on my TODO list, outside of this PR. Thanks for the heads-up

[rcassani]

and yes we can merge the motion latter using the GUI or process

Sounds good

[rcassani]

@Edouard2laire, two changes in 83b10d0:

1. With your recent changes, if an event was not channel-wise and was present in only on file. Once combined, this event was also not channel-wise, so it suggested that it applied to all channels, not only for channels of the file that had the event. This is now fixed.

2. The fact that duplicate names started in _02 was bugging me. Now duplicated events are _01, ... So the intuitive thing to do at the moment of merging will be to set the event label to the name without the numbers.

[Edouard2laire]

The fact that duplicate names started in _02 was bugging me. Now duplicated events are _01, ... So the intuitive thing to do at the moment of merging will be to set the event label to the name without the numbers.

yes, me too. but i was not sure on the best way to fix that :) Glad you have done it :)