Closed ftadel closed 2 years ago
HosseinShahabi
commented
5 years ago Hi @martcous @ftadel I'm trying to add an image to the tutorial. I uploaded the image in different formats and used the script to attach to the body but it doesn't work. Would you please take a look at it? Also, can I upload .png files?
Thanks
ftadel
commented
5 years ago The button to insert images doesn't work in the GUI editor: you have to first upload the file on the page (Menu > Attachments), then use the syntax {{attachment:filename.gif}}
GIF format recommended for interface screen captures (smaller), but you should be able to use any file format supported in web browsers (including png)
ftadel
commented
5 years ago New questions on the forum:
Message from Richard, 20 Jun 2019:
In an effort to move things along faster we will have Takfaranias working with Hossein to complete tutorials and software for Connectivity on BST and Juan working with John, me and Dimitrios to address the stats issues we touched on last week.
ftadel
commented
5 years ago More connectivity questions: https://neuroimage.usc.edu/forums/t/frequency-bands-for-granger-causality-spectral/12300/2
tmedani
commented
5 years ago Hi,
@martcous @ftadel @HosseinShahabi
I noticed a small issue with the connectivity graph
=> 'Display as graph [NxN]'
The issue is in the case of high number of electrodes or scouts,
in this case the size of the police is too small and we can not read the name of the node.
The zoom option is not flexible and it's not possible to choose a ROI.
I found the related function for this (view_connect) but I did not find how to fix it for now.
Is it possible to use similar functions available as in the "electrodes display" or at least an access to the Matlab controls menu. Any propositions or orientation to the right function ?
Thanks
martcous
commented
5 years ago Thank you for the report, Takfarinas.
I enabled being able to move around the graph (translation) with shift + click and mouse move on the following commit: https://github.com/brainstorm-tools/brainstorm3/commit/53af31cc05c7ef0fc3aa581ebe4fe64d42f40fe5
You can also zoom in and out using the mouse wheel or up and down arrows. The translation is not perfect but I tried to improve it slightly. Let me know how it goes!
Martin
tmedani
commented
5 years ago Thank you for the report, Takfarinas.
I enabled being able to move around the graph (translation) with shift + click and mouse move on the following commit: 53af31c
You can also zoom in and out using the mouse wheel or up and down arrows. The translation is not perfect but I tried to improve it slightly. Let me know how it goes!
Martin Hi Martin, Thanks for the quick answer. I modified the code locally and it works as explained. It's much better than before. We have more flexibility on the graph.
Thanks Takfarinas
ftadel
commented
5 years ago The best would probably be to recode all this in pure Matlab code, and get rid definitely of Java3D. Martin: Wasn't this something you wanted to work on?
martcous
commented
5 years ago I've done some work in optimizing the graph display when implementing the fibers connectivity display and since then I feel like it works quite well. I did not plan on re-writing it entirely in Matlab, no.
HosseinShahabi
commented
5 years ago @ftadel @martcous Hi guys, @tmedani and I are working on documentation and codes.
I'm wondering if it is possible to have a figure like transfer function. We have spectrum right now but we like to have it in this shape: https://neuroimage.usc.edu/brainstorm/Tutorials/Connectivity#Simulated_data_.28AR_model.29
I have the function in Matlab but definitely not in the Brainstorm figure format. Any thoughts? Thanks so much.
ftadel
commented
5 years ago Is it to display a by-product of the analysis, or the full final results?
Everything is doable in terms of interface. But coding interactive figures that look like what you show here would take maybe one or two weeks full time to develop and debug. Please double-check with @richardmleahy that is really where you want to go in terms of connectivity display.
If you want to start developing this, we need to set up meetings to design the specification of such figures (what happens when we click where, how do the figure gets refreshed, how does it interact with the rest of the environment...)
HosseinShahabi
commented
5 years ago @ftadel
Regarding the Coherence "process" and "bst" functions to avoid inconsistency or reproducibly or ... I plan to do this: Design a new process function (Only NxN with Static/Dynamic options to avoid having several GUIs) based on our new requirements and definitions. It will be recognized for instance as "hcoh-fft-2019' in other codes. We will keep the current ones but say it is not recommended anymore
For bst function: I edit the current "bst_cohn.m" in a way that produces both results (old and new) based on the method (process_function). Something like bst_bandpass_hfilter that computes both former and newer implementations.
Let me know if this works.
ftadel
commented
5 years ago Sounds good.
Only NxN with Static/Dynamic options to avoid having several GUIs
The 1xN processes are also convenient, and not a lot of extra work. If you're planning on replacing the coherence processes, I guess you would need to replace 3 process functions (process_cohere1, process_cohere1n, process_cohere1n_time, process_cohere2, process_cohere2_time) with 3 new functions (eg. process_cohere2019_1, process_cohere2019_1n, process_cohere2019_2)?
Once the new ones are fully functional, the old process functions can be moved to: brainstorm3/toolbox/process/deprecated
HosseinShahabi
commented
4 years ago I checked the available function for Amplitude Envelope Correlation. The code is a bit different from what I wrote.
It computes the norm of frequency-domain signals, so in case the values are very small, it substitutes the Correlation with zero or other terms. For this reason, the results between the two functions are not exactly similar.
In case we want to remove the available function, we will face the problem of reproducibility. Any suggestions?
ftadel
commented
4 years ago I think we should not keep two versions of the same measure in the software, a choice has to be made for keeping one or the other. Please bring this topic at the next meeting.
Regarding reproducibility, we can move the previous process .m files to the folder brainstorm3/toolbox/process/deprecated so that it is still usable in existing scripts, but not listed anymore in the available processes in the GUI.
HosseinShahabi
commented
4 years ago Okay. I guess the problem is solved and the two shows almost identical values for the connectivity. Here is the comparison of concatenated values. (Envelope Correlation with Orthogonalization)
ftadel
commented
4 years ago The new HENV functions were merged in this PR: https://github.com/brainstorm-tools/brainstorm3/pull/323
The 1xN (process_henv1.m) and AxB (process_henv2.m) versions of the process are missing.
These are important for computing the measure between two different files (eg. one signal from one file vs. some signals in another file), two different modalities (between one recorded signal and the estimated sources) or to obtain simpler displays and faster computations for people who are only interested with the connectivity of one signal of interest with the rest of the signals in the file.
This requires the reformatting of bst_henv.m to accept two sets of signals in input, similarly to bst_cohn.m.
When all the HENV functions are available, should the functions process_aec*.m be moved to the folder "deprecated? Could you add an advanced section in the tutorial to explain how to reproduce the previous AEC results with the new HENV process?
ftadel
commented
4 years ago ?
HosseinShahabi
commented
4 years ago @ftadel I'm currently working on a paper but, in a few days, I will update these functions and Tutorial based on your request. You can also close the previous pull request. Thank you
ftadel
commented
4 years ago Any update on this front?
I just spotted a message that was never addressed on the forum: https://neuroimage.usc.edu/forums/t/problem-with-time-dynamic-envelope-correlation/21542
ftadel
commented
4 years ago @HosseinShahabi @richardmleahy Is this moving forward?
HosseinShahabi
commented
4 years ago The new HENV functions were merged in this PR: #323
Missing processes [1xN] and [AxB]
The 1xN (process_henv1.m) and AxB (process_henv2.m) versions of the process are missing.
These are important for computing the measure between two different files (eg. one signal from one file vs. some signals in another file), two different modalities (between one recorded signal and the estimated sources) or to obtain simpler displays and faster computations for people who are only interested with the connectivity of one signal of interest with the rest of the signals in the file.
This requires the reformatting of bst_henv.m to accept two sets of signals in input, similarly to bst_cohn.m.
What to do with the previous AEC processes?
When all the HENV functions are available, should the functions process_aec*.m be moved to the folder "deprecated? Could you add an advanced section in the tutorial to explain how to reproduce the previous AEC results with the new HENV process?
I'm working on this now. As you said, it needs to reformat the inputs for bst_henv.m. I can change it but I don't know how does it affect those who already used it? I can put the second data in "OPTIONS" so we don't need to reformat the inputs. Your thoughts?
ftadel
commented
4 years ago I'm working on this now. As you said, it needs to reformat the inputs for bst_henv.m. I can change it but I don't know how does it affect those who already used it? I can put the second data in "OPTIONS" so we don't need to reformat the inputs.
The only function calling this function is bst_connectivity, and only in the context of process_henv1n. Therefore bst_henv can be considered an internal function, for which we don't have to keep any backward compatibility. You can change the list of input parameters of the function to something like (X, Y, Time, OPTIONS).
Thanks!
HosseinShahabi
commented
4 years ago Okay great
HosseinShahabi
commented
4 years ago @ftadel Question: What is the time vector in case AxB? Should we assume the user inputs the data with the same length and channel info and wants the connectivity between two conditions for the selected channel?
I added two datasets in Files A and B with different lengths and it computed the amplitude envelope correlation without error!
ftadel
commented
4 years ago Question: What is the time vector in case AxB? Should we assume the user inputs the data with the same length I added two datasets in Files A and B with different lengths and it computed the amplitude envelope correlation without error!
In the coherence process, if you select files with different time vectors, you get an error. Does it make sense to compute AEC from files of different dimensions? If it does not mean anything, please add an error message to prevent this from happening. If it's OK, please add a note in the comments in the header of the function to indicate input data matrices can have different dimensions. Same goes for all the other connectivity functions.
and channel info and wants the connectivity between two conditions for the selected channel?
No, they can be files of different nature: eg. EMG vs. source maps, EEG sensor versus scouts. They can come from the same folder/subject or from different folders/subjects.
HosseinShahabi
commented
4 years ago @ftadel Well, for me there was no error for all measures including Coherence, in case file A is shorter than B, but an error vice versa. So I had to check why this happens.
Here, we check the length of two files: https://github.com/brainstorm-tools/brainstorm3/blob/3468d38ec983c4200152b26a578db858d49089db/toolbox/connectivity/bst_connectivity.m#L223
But file B was loaded in this line using the general and only Options.TimeWindow, which belongs to file A https://github.com/brainstorm-tools/brainstorm3/blob/3468d38ec983c4200152b26a578db858d49089db/toolbox/connectivity/bst_connectivity.m#L212
So when file A is shorter only that part of file B will be loaded and sizes will be the same. If A is longer, there are not such samples in B, so we see an error.
Do you have any idea how/where it should be fixed? or is it necessary to be fixed at all? Thanks
ftadel
commented
4 years ago But file B was loaded in this line using the general and only Options.TimeWindow, which belongs to file A
In the process options, the TimeWindow option are not "for filesA", it is for both A and B. It happens to be set by default to the duration of A, but this is only a default value, that users should double-check and are able to change.
It could be a valid case to have FilesA defined for [-2,+2]s, FilesB defined for [-3,+3]s, and that you want to compute a connectivity measure only on [-1.5,+1.5]s for all the files.
It could be confusing for the user who might suppose, by using [-2,+2]s in the process options, that it would use the entire duration of A and B, and in reality it would use only [-2,+2]s for both A and B. Too bad. It can't lead to dramatic misunderstanding either.
The test just below makes sure that the same amount of data was read from both lists, so a meaningful error would always be triggered: https://github.com/brainstorm-tools/brainstorm3/blob/3468d38ec983c4200152b26a578db858d49089db/toolbox/connectivity/bst_connectivity.m#L222
Do you have any idea how/where it should be fixed? or is it necessary to be fixed at all?
I think what is happening at the moment is OK, case closed :)
ftadel
commented
3 years ago Option to process scouts "before" doesn't seem to work: https://neuroimage.usc.edu/forums/t/problem-with-time-dynamic-envelope-correlation/21542/7
HosseinShahabi
commented
3 years ago I used to extract time series (scout) and then simply run the function one a manageable dimension (usually less than 200 channels). But what does before and after mean here?
Like for Before: 15000 dipoles and first compute a 15,000 by 15,000 connectivity matrix and then average among matrices? That cannot work. Or should the function itself compute the final time series for the scout and then connectivity value.
I'm using the old AEC either but it also shows dimension 45,000 by 45,000 so I had to stop it.
ftadel
commented
3 years ago But what does before and after mean here?
The same as everywhere else: https://neuroimage.usc.edu/brainstorm/Tutorials/TimeFrequency#Scouts
Like for Before: 15000 dipoles and first compute a 15,000 by 15,000 connectivity matrix and then average among matrices?
The option "before" of the old AEC process does what it is supposed to: it applies the scout function first, and then compute the connectivity measure only from the scouts time series.
The new process Envelope correlation NxN (2020) ignores the scout selection.
One line at the beginning of the Run function deletes the scout selection information.
Before line 111, OPTIONS.TargetA contains the list of scouts: {'User scouts', {'scout1', 'scout2'}}.
Then the line 111 OPTIONS.TargetA = OPTIONS.TargetB; resets it.
This is related with the question of the 1xN / NxN / AxB process, and one of the reasons for which all these process exist.
ftadel
commented
2 years ago Let's close this outdated issue. We'll open new ones when we identify new problems.
For now, let's rely on the TODO list of the new connectivity tutorial: https://neuroimage.usc.edu/brainstorm/Tutorials/Connectivity#TODO
Waiting for updates from Richard and Hossein.
This tutorial was started a few years ago and never actually written: https://neuroimage.usc.edu/brainstorm/Tutorials/Connectivity
There is also this page where I was trying to centralize the summary of some of the discussions: https://neuroimage.usc.edu/brainstorm/ConnectivityAdmin All the information in there should be either deleted (if of no interest anymore), moved to this issue (for topics that still need to be discussed), or moved to the connectivity tutorial (for info relevant for users).
A section "Connectivity" is also available in the general todo-list: https://neuroimage.usc.edu/brainstorm/Next#Connectivity
TODO: