ENH: apply_forward; compute sensor space data from source estimate #21
Conversation
|
|
||
| examples_folder = op.join(op.dirname(__file__), '..', '..', 'examples') | ||
| data_path = sample.data_path(examples_folder) | ||
| fname = op.join(data_path, 'MEG', 'sample', 'sample_audvis-meg-oct-6-fwd.fif') | ||
|
|
||
| fname_raw = op.join(data_path, 'MEG', 'sample', 'sample_audvis_raw.fif') |
There was a problem hiding this comment.
maybe you can use the tiny raw file in fiff/tests it will be faster
| times = deepcopy(stc.times[start:stop]) | ||
| ch_names = deepcopy(fwd['sol']['row_names']) | ||
|
|
||
| return data, times, ch_names |
There was a problem hiding this comment.
what I don't really like about returning not an Evoked object is that there is no convenient way to work with such data, times, ch_names. You cannot really plot it and don't have access to measurement info to find channel type and so on. Why not passing an info parameter and eventually an evoked_template?
There was a problem hiding this comment.
Thats's true, the function doesn't return information such as channel types etc. My reason for having two functions (apply_forward and apply_forward_raw) was that the first doesn't require a template. So it is easier to use if one just wants to project source space data to the sensor space without wanting to store the result or visualize the channels.
How about adding a 3rd function apply_forward_evoked, which does the same thing as apply_forward_raw but for Evoked?
There was a problem hiding this comment.
I had an idea during the night. Basically data, times, and ch_names are attributes of an Evoked object. The Evoked has just an extra info attribute for the measurement info. So one option is to have BaseEvoked object and an extra info param in apply_forward. If info is not None you return an Evoked and if info is None then you return a BaseEvoked object that is just a namedtuple with 3 attributes (data, times and ch_names). Makes sense?
There was a problem hiding this comment.
I think this will make things confusing to the user. As BaseEvoked wouldn't have channel and measurement info, it wouldn't be possible to plot the sensor topography etc. Users are also likely to pass BaseEvoked to functions that expect Evoked and will be confused if it doesn't work.
There was a problem hiding this comment.
ok however that would avoid having 2 different functions for simultating Evoked and plain data. What is not clear to me is what you can actually do from plain data. You would not even be able to apply an inverse operator.
There was a problem hiding this comment.
True, let's just have 2 functions, apply_forward_raw and apply_forward_evoked.
There was a problem hiding this comment.
True, let's just have 2 functions, apply_forward_raw and apply_forward_evoked.
maybe i am neat picking but if apply_inverse is not named
apply_inverse_evoked we should have apply_forward that works with an
Evoked instance… :)
|
Ok, now apply_forward returns Evoked, so it is consistent with apply_inverse. |
|
one last thing before you want to kill me ... To me having a template and include, exclude parameters is a bit redundant. I would expect the output Evoked to have the same channels and the function to raise an error if a data channel in Evoked is not present in the forward. A typical use case would be to keep only the MEG channels and I don't feel it would be very convenient with include, exclude. I'd rather do first a pick_channels_evoked and then the apply_forward. I'm just trying to avoid having too many different ways to do the same thing. If you're fed up I can do it tomorrow if you want. |
|
True, it is a bit redundant. At the moment, the returned Evoked/Raw object will have all the channels that are present in fwd, as the channel info is copied from there. The problem with using pick_channels_evoked is that the corresponding function doesn't exist for Raw (and it would be difficult to implement, as the data usually stays on disk). How about we use pick_channels_forward and pick_types_forward (I can add this function)? |
|
ok this makes things even simple. We had the pick forward functions |
|
Yes, it will be nice and clean :). Regarding raising an exception, isn't all the channel info included in fwd['chs']? At the moment I copy the channel information from there, so it should work even if a channel isn't present in the info of the template. Right? |
I like that !
it should but you never know what users do :)
I think it shouldn't |
|
Another attempt ;-) |
|
maybe you can avoid the duplicated code in the tests and then press the green button ! thanks heaps ! |
ENH: apply_forward; compute sensor space data from source estimate
|
merged ! I'll polish the tests |
|
done ! |
|
thanks |
This is useful, e.g., when evaluating methods or when working with surrogate data in source space.