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

Fix Extrapolation #20

Merged
merged 28 commits into from
Aug 30, 2022
Merged

Fix Extrapolation #20

merged 28 commits into from
Aug 30, 2022

Conversation

SimonDanisch
Copy link
Member

Combine the approaches from #15 #12 and adds comparison tests with PyMNE.
Still needs some clean up of the API and documentation + comments.

@SimonDanisch
Copy link
Member Author

Ugh, I knew that a python test dependency would be trouble^^

The Python package matplotlib could not be imported by pyimport. Usually this means
that you did not install matplotlib in the Python version being used by PyCall.

PyCall is currently configured to use the Python version at:

/usr/bin/python3

and you should use whatever mechanism you usually use (apt-get, pip, conda,
etcetera) to install the Python package containing the matplotlib module.

@SimonDanisch
Copy link
Member Author

Now, this adds a new geometry extrapolation, which adds points from a geometry (right now Rect + Circle), and then uses ScatteredInterpolation with Shepard distance (basically just a mean weighted by the distances to the existing points) to give those extra points values.
The code is here

# We use the original new points from boundingobx and not the extended one, so that the distances aren't becoming way too big.
(linked the likely most problematic line).

I also added some tests for creating those points:
Red: Bounding geometry
Blue: Extended geometry to minimize interference (took the 3x blow up from mne):
image
image

I also added new tests to directly compare the output to MNE.
Format is [TopoPlots.jl vs PyMNE] and if the scatter marker are very visible, it means a peak shift... I think we now do pretty well!
Circular extrapolation:
image
Rectangular extrapolation (like mne):
image

Problematic peaks:
image

Standard data:
image

@behinger
Copy link
Collaborator

wow! looks really good. Very impressed about the integration of matplotlib plots...

I have a quick question: for me ScatteredInterpolation.jl was super slow (#5). Did you observe the same, or is it negligible because you only interpolate few points?

@SimonDanisch
Copy link
Member Author

Ah come on...why is there always a different error with the python deps every time there's a new CI run?
image

@SimonDanisch
Copy link
Member Author

@behinger good question...
I haven't benchmarked it, but I was pretty sure that it should easily be fast enough for the few points, since it just calculates a couple of distances.
I imagine that it would be quite different for a 512x512 matrix of points to interpolate, which should be fairly slow!

@SimonDanisch
Copy link
Member Author

Added a graphic to the docs to explain the extrapolation:

test

Project.toml Show resolved Hide resolved
test/runtests.jl Outdated Show resolved Hide resolved
src/core-recipe.jl Outdated Show resolved Hide resolved
src/core-recipe.jl Outdated Show resolved Hide resolved
function mne_topoplot(fig, data, positions)
circle = TopoPlots.enclosing_geometry(Circle, positions)
# Seems like the only way to plot positions with matching head is to norm the positions..
# Anything else leads to anarchy!
Copy link
Collaborator

Choose a reason for hiding this comment

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

how does this relate to #18?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question... I don't think it does, but I don't know MNE well enough, to know what's going wrong on there side.
I'll take a look at #18 after this PR!

@palday palday mentioned this pull request Aug 30, 2022
@palday palday merged commit b32c1b5 into master Aug 30, 2022
@palday palday deleted the sd/fix-extrapolation branch August 30, 2022 20:16
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