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: is_surface() for volume source spaces #511

Merged
merged 6 commits into from Mar 12, 2013

Conversation

larsoner
Copy link
Member

Addresses issue on mne_analysis listserv. Pending tests passing here, I plan on merging to fix the person's issue since this one's a quick fix.

@larsoner
Copy link
Member Author

@agramfort has successfully instilled a fear in me of merging my own PRs, so I'll actually wait for someone to review before merging :)

@agramfort
Copy link
Member

LGTM but maybe we should wait for Jon to get his sample vol.stc file. FYI I came up with the -vl.stc myself (2 chars like lh or rh).

@jhouck
Copy link
Contributor

jhouck commented Mar 12, 2013

I sent the vol.stc last night -- did that not come through? As you had guessed, it was produced via mne_sensitivity_map with a volume forward model. The attached jpeg is that same stc file with a .jpg extension, which you may or may not be able to download from https://f.cloud.github.com/assets/857583/249528/bd8468b6-8b2f-11e2-8a5f-d33202bc6449.jpg.
sample_sensitivity_map_mag-vol stc

The syntax to create the file (in 2.7.4, rev 3394) was:
mne_sentivitity_map --fwd sample_audvis-meg-vol-7-fwd.fif --map 1 --mag --stc ${SUBJECT}_sensitivity_map_mag
where sample_audvis-meg-vol-7-fwd.fif had been created using the tutorial script (run_meg_volume_tutorial.sh).

@larsoner
Copy link
Member Author

@agramfort looks like -vol.stc and -vol.w are defaults for MNE-C. I added the following line to mne-scripts volume tutorial:
https://github.com/mne-tools/mne-scripts/blob/master/sample-data/run_meg_volume_tutorial.sh

mne_sensitivity_map --fwd sample_audvis-meg-vol-7-fwd.fif \
    --map 1 --w sample_audvis-grad-vol-7-fwd-sensmap

This generates the file sample_audvis-grad-vol-7-fwd-sensmap-vol.w. Thus I think it makes sense for us to support both *-vol.* files, as well as volume .w files (neither of which were supported before), since that's what MNE-C produces in this case. Latest commit adds these features, with tests. Let me know what you think.

@larsoner
Copy link
Member Author

@jhouck I was able to produce a similar sensitivity map using the sample dataset. My PR /should/ allow you to use your files. If you'd like to give it a try, you can do the following in your mne-python directory:

$ git remote add Eric89GXL git://github.com/Eric89GXL/mne-python.git
$ git fetch Eric89GXL
$ git checkout -b vol-fix Eric89GXL/vol-fix

And then install as you normally would. If you could take a crack at it to see if it fixes your issues, that would be helpful.

@jhouck
Copy link
Contributor

jhouck commented Mar 12, 2013

@Eric89GXL Sure, I'll give it a shot.

agramfort added a commit that referenced this pull request Mar 12, 2013
FIX: is_surface() for volume source spaces
@agramfort agramfort merged commit 559b0b0 into mne-tools:master Mar 12, 2013
@agramfort
Copy link
Member

I've updated the build bot with the new test file. Thanks !

@larsoner
Copy link
Member Author

@jhouck now that it has been merged from master, in case you aren't too familiar with Git, you should be able to do something like:

$ git checkout master
$ git fetch upstream # or "git fetch origin" if you have the main repo named that way
$ git pull upstream master

Then install as usual.

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.

None yet

3 participants