-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add TFR vector visualization #6
Open
alexrockhill
wants to merge
7
commits into
mne-tools:main
Choose a base branch
from
alexrockhill:vector
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
c9992fd
Add TFR vector visualization
alexrockhill f2d105e
fix style, try to fix 3D rendering
alexrockhill 1b70bc8
MAINT: Make implicit req explicit
larsoner 9fa75a7
FIX: Black
larsoner afe2c34
rerun cis
alexrockhill 213016a
FIX: CIs
larsoner e8de33f
FIX: CIs
larsoner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ dependencies = [ | |
"qtpy", | ||
"pyvista", | ||
"pyvistaqt", | ||
"matplotlib", | ||
"mne", | ||
"nibabel", | ||
"dipy>=1.4", | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is equivalent to
data.real
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, so that was just plotting the real component, that's even simpler to understand potentially. What do you think about interpreting this? The real component is just the amplitude of the cosine for that frequency compared to what I was originally thinking was tapering the magnitude of the vector by it's alignment with 0 or 180 degree phase. Since those are equivalent, I'm thinking there might be a simpler interpretation, I guess that the cosine is already zero at 90 and 270 degrees so the real part already includes that tapering.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a hard time interpreting the real or imaginary parts of these filters. In spectral analysis it means some cosine or sine part of a complex exponential (it's just a convenient way of bookkeeping). And in cross-spectrum / coherence analysis it means some phase shift between signals of interest. So plotting just the real (or imag) part of the signal only gives you part of the information. I don't know the right thing to do here but plotting just the real part (or just the imaginary part) seems to miss a lot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm those are good examples of where the real and imaginary parts of the filters are used directly but I think via the Hilbert transform, the imaginary part of the signal is just the real signal shifted in phase by a quarter cycle so I think that way the information is actually redundant and you might not be missing anything after all. In this gist (https://gist.github.com/alexrockhill/58c4316415c54fcc24f85a617862e2bf) I simulate a pure sine wave and then plot it via this method and similarly in the code below, you what I would expect to recover; a vector oscillating in the direction that was simulated. If there is a reason why visualizing it this way and this interpretation is wrong, I'm happy to abandon the idea or table until we do it some other way that is better, but this is what I was thinking would be helpful to visualize--when you get a pure oscillation, the vector changes direction forward and backward on the brain in the direction of that oscillation, I think the math on how to get there changes the interpretation but I don't quite understand why this isn't helpful or what is wrong about this visualization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Same can be said of the real and imaginary parts of the Fourier basis (cos +j sin).
No, I don't think this is how the Hilbert transform is meant to be interpreted/thought of in practice in signal processing any more than you can think of the real and imaginary coefficients of a Fourier decomposition as being redundant (the bases are actually orthogonal). And it becomes more complicated once you're talking about it in the context of a cross special density estimate, which underlies DICS, which is where I assume this would actually be useful. I think a deeper understanding of the underlying signal processing principles at play here (ideally derived from math) should guide any public implementation here rather than simulations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The imaginary part isn't totally redundant, but it is similar to the phase-shifted real component in actual data. The cosine of the phase scaling the magnitude is identical to the real value in the experimental data also.
As I understand it, the cross-spectral density is just for whitening and the complex data itself is what gets input to the minimum norm or beamformer solutions.
I don't want to post hoc justify that the real component is what is the right thing to visualize when the phase-scaled magnitude was what I was going for and was theoretically motivated but it would also be nice to visualize the direction. I know a first-component-of-the-SVD method has also been proposed which would be interesting to see implemented but it would also be nice to plot something that isn't just the direction of maximum variance if possible. Since this gets a bit complicated to understand and interpret, it would be nice if there was something with minimal processing of the data. The real component is as minimal as it gets but obviously might be too simple and not the right solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for a good discussion, how about we table it for now and come back to it later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we can discuss it later