Proposal: VertexPickerMixin

[wmvanvliet]

While implementing the new DICS beamformer pipeline, one of the steps is to optionally restrict the source space to vertices with a maximum distance to the nearest sensor (to remove deep sources). This can be implemented by restricting either the SourceSpaces, or Forward objects.

Right now, there are a number of ways to "pick vertices" of a Forward or SourceEstimate object (restrict_forward_to_label, restrict_forward_to_stc, SourceEstimate.in_label). Rather than adding a restrict_forward_to_sens_dist function, I could instead create a VectexPickerMixin mixin that implements .restrict_to(vertices), restrict_to_label(labels), restrict_to_stc(stc), restrict_to_sens_dist(info), etc. and have SourceSpaces, Forward and SourceEstimate subclass this.

How do you feel about this? Maybe it deprecates too much API...

[larsoner]

It would be nice to unify with methods rather than using a fragmented / incomplete set of functions. Let's see if it passes the @agramfort test, but I'm +0.5

[wmvanvliet]

We can have an additional set of functions if we want, such as:

restrict_forward_to_label()
restrict_src_to_label()
...

they would be one-liners with a @copy_function_doc_to_method_doc decorator.

[agramfort]

I would be in favor of plain functions for now. Following what you suggest it would be restrict_forward_to_vertices rather than restrict_forward_to_sens_dist having these consistent restrict_* functions sounds good me.

[wmvanvliet]

What we need for the DICS pipeline is a way to obtain a list of vertno's of vertices that are no further away than 7cm from the nearest sensor. This list needs to be created on one subject, saved, and subsequently used for all other subjects.

With that use case in mind, I propose the following API:
https://gist.github.com/wmvanvliet/b6f6acbeba26642f389028509c56caf7

Proposed functions

select_vertices_in_label(inst, label):
    """Return the vertno of the vertices that lie within one of the labels."""
    
select_vertices_in_sensor_range(inst, max_dist, info=None):
    """Return the vertno of the vertices which nearest sensor is no further away than
       max_dist."""
    
restrict_vertices(inst, vertices):
    """Restrict the vertices in a Forward, SourceSpaces or SourceEstimate object to
       the given vertno."""

Existing API can be implemented using the above functions

# restrict_forward_to_stc(fwd, stc)
restrict_vertices(fwd, stc.vertices)

# restrict_forward_to_label(fwd, labels)
restrict_vertices(fwd, select_vertices_in_label(fwd, label))

# src.in_label(labels)
restrict_vertices(stc, select_vertices_in_label(stc, label))

But the new API would also allow stuff like this

restrict_vertices(src, stc.vertices)
restrict_vertices(fwd, select_vertices_in_sensor_range(fwd, max_dist=0.07))
restrict_vertices(stc, select_vertices_in_sensor_range(stc, max_dist=0.07, info=info))

[wmvanvliet]

The idea is then to keep the existing API as is, but we re-implement it using the new API. Then, if more ways of restricting a Forward | SourceSpaces | SourcEstimate are desired, they can be implemented using the new API.

[agramfort]

looks clean to me in terms of API but it's the first time I hear someone doing this :) So I am really curious to see the impact of this by replicating your pipeline without it :)

[wmvanvliet]

The idea is that all-to-all connectivity on the source level gets messy really quick. You want to reduce the number of vertices as much as possible. One way is to decide not to care about deep sources, as they will be unreliable anyway. (Other ways we employ is to use ico3 and to only compute connectivity between pairs of vertices at least 4cm apart)

[larsoner]

Some of the vertex picking/restriction is refactored a bit in #6037, so I'd recommend not starting on this issue until we merge that PR or at least pull out the relevant restriction parts from the code. Happy to work on this at the sprint