[BUG/MAINT] Topomap `image_interp='None'` still interpolates

[alexrockhill]

In writing https://github.com/mne-tools/mne-python/pull/10571, having values that are not spatially continuous over a topomap made for real problems with the existing interpolation methods so I added a Voronoi option. The following code uses image_interp='None' but in my mind, this is still applying an interpolation. I was planning on adding the Voronoi option to image_interp but I thought maybe it should replace None. In the matplotlib documentation, the interpolations are demonstrated on images, which makes much more sense for None, but with points, I think this behavior is unexpected--for me, I just want an option to see the raw data. I added a new opinions-wanted tag because I think this is an especially good one to hear what other people think.

import os.path as op
import mne
import matplotlib.pyplot as plt

data_path = mne.datasets.sample.data_path()
evoked =mne.read_evokeds(op.join(data_path, 'MEG', 'sample', 'sample_audvis-ave.fif'))[0]
evoked.pick_types(eeg=True)
evoked.apply_proj()
fig, ax = plt.subplots()
mne.viz.plot_topomap(evoked.data.mean(axis=1), evoked.info, image_interp='None', axes=ax)   # still interpolates

The Voronoi plot looks like this:

[alexrockhill]

I just added it as an option under voronoi in the referenced PR, this issue would be to use it for image_interp='None' as well.

[mmagnuski]

The Voronoi topomap is cool! For matplotlib the documentations mentions interpolation='none', not 'None', but I didn't test if this make a difference.

[mmagnuski]

And yes, I think the Voronoi option is a great default no-interpolation, but whether it should be image_interp=False or image_interp='none' (etc.) - I have no strong opinion.

[alexrockhill]

The image_interp='none' is the first topoplot, where it actually still has interpolation because the points are set as interpolation points. That's a bit confusing to me at least.

[mmagnuski]

@alexrockhill This is an outcome of two successive interpolations:

• mne sets up the interpolation points grid and interpolates the values (channels -> rectangular array)
• the interpolated grid is then sent to matplotlib's imshow, and matplotlib can further interpolate depending on image_interp (rectangular array -> image)

You can see this better by decreasing res and manipulating image_interp:

fig, ax = plt.subplots(ncols=2)
for axis, interp in zip(ax, ['bicubic', 'none']):
    mne.viz.plot_topomap(evoked.data.mean(axis=1), evoked.info,
                         image_interp=interp, axes=axis, res=12,
                         show=False)

But this can be counterintuitive indeed, I just never used image_interp argument so I never faced it.

[alexrockhill]

Exactly, my point is that the underlying image is probably best thought of as the Voronoi diagram which is then interpolated rather than a grid of squares, especially since the squares are not well aligned with the data.

[mmagnuski]

I'm not sure if I understand you well. I agree that if one does not want to perform interpolation than the output should be the Voronoi plot. However, I think that this should be done on the side of interpolation done by mne (which for example sets the outer extrapolation points in various ways depending on extrapolation and border arguments), not the aesthetical postprocessing in matplotlib's imshow controlled with image_interp. Maybe we just need another argument, interpolation for example, since we already have extrapolation.

[alexrockhill]

That sounds pretty reasonable, it ends up being a much larger API change though... my really main point is that when you pass interp_image='none' I would expect no form of interpolation at all and currently that's not what happens.

[mmagnuski]

my really main point is that when you pass interp_image='none' I would expect no form of interpolation at all and currently that's not what happens.

Yes, I agree there should be an intuitive option to turn off interpolation completely (currently interp_image='voronoi'). But, given how mne does the interpolation, using interp_image for the voronoi option is not very accurate. Another thing is that 'voronoi', as an arugment to interp_image, by itself is not very descriptive, so something like interpolation=False (or anything else that uses False, None or 'none') would be a better option, I think. BTW, currently even if you specify interp_image='voronoi' the grid interpolation is done anyway, it is just thrown away. This is the part of your PR that I didn't pay enough attention to, sorry. I think we can wait for others (@larsoner @agramfort @cbrnr ) to cast their votes. :)

[agramfort]

Can you offer me a concise list of the vote options with what they imply in terms in API and deprecation? 🙏

[alexrockhill]

I think it's just whether to replace image_interp='None' with a Voronoi diagram. The other suggestion was to add separate interpolation argument that does the same thing as image_interp to fully depreciate but I think it's fine to just change it without depreciation because I think the behavior is unexpected/incorrect.

[mmagnuski]

@agramfort @alexrockhill I think the issue is a bit more complex - I don't think image_interp should be doing the Voronoi diagram, because it is an argument that allows to control the interpolation only on the matplotlib's side. But before that, the actual interpolation is done on the mne side, and only then the interpolated image is passed on to matplotlib, which does additional interpolation as part of plt.imshow, mostly for easthetic reasons (so that the pixelation of the interpolation grid is not seen). Only this later interpolation gets the image_interp argument, but at this point the interpolation has already been done by mne, so image_interp shouldn't be able to turn it off.

So, my point is that to control the whole process of interpolation, and not only the part done by matplotlib, we need something else than image_interp, for example interpolation keyword (to follow extrapolation argument we already have). This does not mean that image_interp should be deprecated, it should retain its role of controlling matplotlibs imshow interpolation behavior. But to turn off the interpolation completely and get the Voronoi diagram, I think we'd need another argument.

[alexrockhill]

I thought interpolation would supercede image_interp, wouldn't it be a bit confusing/redundant to have both?

EDIT: Or then interpolation would be a bool argument? It seems like it could encompass both a matplotlib interpolation and other non-matplotlib options that could be extended in the future.

[mmagnuski]

I thought interpolation would supercede image_interp, wouldn't it be a bit confusing/redundant to have both?

I agree it may be confusing, but this issue uncovers that the image_interp argument is confusing as well. At this point two things can be done, I think:

• clarify that image_interp does not control interpolating the channels signal but just "post-processing" done by plt.imshow on the interpolated grid created by mne, and that if one wants to turn off interpolation at all, interpolation arguments should be used
• introduce interpolation agument that currently is just boolean and controls whether the interpolation is performed (current cubic interpolation), or not (nearest interpolation using Voronoi diagram). In future this argument could allow to control other interpolation options on the mne side.

[agramfort]

can I insist and ask why a user may want to use a given option beyond what we offer by default?

Message ID: @.***>

[alexrockhill]

For the electrode bridging, having any kind of interpolation was misleading because the values were not continuous in topomap space (there would be weird local minimum between electrodes). I imagine there are other cases where interpolation is misleading, or you might just want to see your data without interpolation. In the latter case, if you pass image_interp='None' you still get interpolation. You have to know to pass Voronoi which many people might not be familiar with.

[agramfort]

ok now I understand where you are coming from... so it seems we need 2 modes: raw / uninterpolated and the way we have it now?

what you are saying here should be in a docstring to explain why one should consider deactivating interpolation.

this is being what is preferred action plan?

Message ID: @.***>

[alexrockhill]

I think it's as simple as using the Voronoi diagram for image_interp=None or 'None' or False but @mmagnuski suggests a new interpolation parameter. Although image_interp='None' is a valid matplotlib argument my argument against allowing that is that the rectangular grid is one option of a base to interpolate off of but I think the Voronoi diagram is more representative of uninterpolated topomap data.

[cbrnr]

I think having two parameters that influence topomap interpolation will be very confusing. It seems like @alexrockhill just needs one additional mode which shows the raw data without any interpolated values anywhere. Can we not assign that to a new value of the already existing parameter? I don't mind it being 'voronoi' (it is very technical, but people who need it will know what they are doing), and neither is it a showstopper if it is technically not really a Voronoi diagram. If we already use 'None', I think we should not change the behavior (and introducing 'none' with lowercase n would be too ambiguous). Do we already use None (the NoneType)?

[mmagnuski]

@cbrnr Well, confusingly image_interp does not control the interpolation that we do in mne, it is only an argument passed to matplotlib's imshow that plots the interpolated grid. This is because we always do cubic interpolation, and the interpolated grid is only additionally smoothed by matplotlib so that you don't see the grid granularity. If we allow a 'voronoi' value that is not handled by matplotlib and that controls the actual interpolation we do before imshow (so not the aesthetical postprocessing) - it might be even more confusing, as the user should expect image_interp to control the interpolation done by mne. Users might already expect that, but adding an argument that is not handled by matplotlib's imshow will only increase confusion in my opinion. I think that first of all we should clarify in the docs that image_interp does not control how the actual interpolation is done. If we want to control the interpolation done by mne, we should have an arugment for that.

[mmagnuski]

(so, currently with image_interp='bilinear' we still do cubic interpolation, but the interpolated grid is then smoothed bilinearily)

[cbrnr]

Users probably only care what will be plotted. The most important use cases seem to be interpolated or not. This can be handled by the existing parameter – even if we always interpolate but then discard the interpolated values (and only show the existing values). I agree we should clarify in the docs.

[mmagnuski]

Ok, clarifying how image_interp actually works is something we all agree, I think, and it should be done irrespective of the actual solution we pick here. So the options are: 1) Keep things as they are now - this means that image_interp='voronoi' gives you voronoi diagram, and all other image_interp values do not control mne's interpolation but are passed to matplotlib. Here we'd only need to fix one thing - skip mne's interpolation when image_interp='voronoi' is chosen (currently it is done and then ignored). This way image_interp controls the overall interpolation only for one specific value ('voronoi'), which (at least in my opinion) can still be a bit confusing after docs clarification. 2) As @alexrockhill suggests change image_interp so that value of None or 'None' (or other valid matplotlib no-interpolation option) works just as voronoi. The downside of this is that some funcionality is lost (turning off matplotlib's interpolation, but leaving mne's) and that image_interp becomes more confusing, because some valid matplotlib options are accepted and passed to imshow, but others are (still valid matplotlib) are not. The fix from solution 1) should be done in this case as well. 3) Make sure we split in the API the interpolation that we do (cubic, voronoi) vs the interpolation that is only done as a second aesthetic step by matplotlib (as I proposed). The mne's interpolation would be handled by interpolation argument (just as we have extrapolation) and could later incorporate linear interpolation. The matplotlib's one would still behave the same as now, image_interp (in the longer run the argument name could be changed to something that better suggests postprocessing, not the actual interpolation). The downside is adding another argument, but better functional separation (we don't mix the two interpolation steps) and hopefully also more clarity. 4) Remove image_interp and add interpolation (no one proposed that but it is another way out of the image_interp confusion). When interpolation='cubic' mne would perform cubic interpolation (current default) and the imshow smoothing would be always bilinear (also current default). When interpolation='nearest' the voronoi would be used and no imshow postprocessing (as it does not make sense in such case). This would be a big API change (at least compared to 1, 2 and 3), with deprecations and explosions, but we could hope image_interp is not used often. Another downside would be loss of control over matplotlibs imshow smoothing that we use. But the resulting API would be simple and clear. 5) a varaint of 4 suggested later in the thread by @cbrnr that keeps image_interp but changes its behavior to interpolation from option 4 above.

I hope the description of the options is not too biased. :) But as I wrote before I'm +1 for option 3 (but then still prefer 1 over 2).

current votes

option	vote count	details
1	0
2	0
3	1	mm + 1
4	1.5	cb + 1; mm + 0.5
5	2.25	cb and el: + 1; mm +0.25

A side issue is to consider naming 'voronoi' option something else like nearest (as this the kind of interpolation it performs) as it would be more clear (but this would clash with matplotlib's 'nearest' argument that already works for image_interp if option 1 or 2 are chosen).

[cbrnr]

Why would anyone want to distinguish between data interpolation and image interpolation? I thought the end result is always the topomap, so I never access the actual data (and therefore do not care if that has been interpolated or not).

So I tend to prefer option (4). Alternatively, we could implement the behavior of (4) with out existing image_interp parameter, which would be less breaking I guess.

[mmagnuski]

Why would anyone want to distinguish between data interpolation and image interpolation?

@cbrnr This is what we do now (kind of), but I agree it is not ideal. The reason for current state of afairs is rather technical, I think - we do cubic interpolation on a grid, but do not waste resources to make the grid super-fine-grained (although it can be controlled by res) if it can be linearily smoothed by matplotlib when displayed for a similar visual effect. I actually never used image_interp when plotting topomaps, and I guess many users did not, so maybe option 4 wouldn't be so destructive as it sounds. So you can count me as +0.5 on option 4 as well. :)

[larsoner]

So I tend to prefer option (4). Alternatively, we could implement the behavior of (4) with out existing image_interp parameter, which would be less breaking I guess.

+1 for (4) while keeping the name as (the slight misnomer) image_interp rather than rename to interpolation by deprecation

Another downside would be loss of control over matplotlibs imshow smoothing that we use. But the resulting API would be simple and clear.

If people really need it, there should be something like ax.images[0].set_interpolation(...) that they can do afterward, so I'm not too worried about this

[mmagnuski]

+1 for (4) while keeping the name as (the slight misnomer) image_interp rather than rename to interpolation by deprecation

In such scenario, the behavior of this argument would change so that would likely require deprecation too. And if the behavior changes why not rename? :) But I can live with repurposed image_interp.

If people really need it, there should be something like ax.images[0].set_interpolation(...) that they can do afterward, so I'm not too worried about this

I didn't know that, nice! I didn't expect that interpolation can be changed after plotting, but it indeed works. :)

[larsoner]

In such scenario, the behavior of this argument would change so that would likely require deprecation too.

I'd consider this a bugfix since what we did before was arguably wrong. And with viz functions we are a looser than normal with what we consider a bugfix anyway since it can be a bit subjective...

[alexrockhill]

it is very technical, but people who need it will know what they are doing

I think it might be a good idea to check uninterpolated data every once in a while, maybe if people did this, they might find downsides to the interpolation.

I'm on board with the proposed changes, I can start a PR. Thanks for all the opinions everyone and especially laying out the details really well @mmagnuski.