vsoch / nidmviewer

NIDM Results Viewer
http://vsoch.github.io/nidmviewer/
BSD 3-Clause "New" or "Revised" License
3 stars 3 forks source link

Display excursion set instead of unthresholded statmap #8

Open chrisgorgo opened 8 years ago

chrisgorgo commented 8 years ago

Currently the viewer is displaying the unthresholded statistical map (see https://github.com/vsoch/nidmviewer/blob/master/nidmviewer/sparql.py#L76), and then papaya does some implicit thresholding. This leads to confusion when one of the coordinates is pointing to a voxel that is inside the excursion set, but below the implicit papaya threshold.

vsoch commented 8 years ago

If this requires sparql prowess, might want to bring @nicholsn into the conversation!

nicholsn commented 8 years ago

happy to help... is the idea to set the threshold in papaya using info from the nidm results graph? would that be using a nidm:threshold?

chrisgorgo commented 8 years ago

This would not account for cluster-based thresholding. A more robust strategy would be to display [nidm:'Excursion Set Map'](http://nidm.nidash.org/specs/nidm-results_120.html#section-nidm:'Excursion Set Map'). One caveat is that the spec does not specify if this is a binary mask and if not what are the non-zero voxels. t-values? z-values? p-values? Maybe @cmaumet could clarify.

On Thu, May 26, 2016 at 4:38 PM, Nolan Nichols notifications@github.com wrote:

happy to help... is the idea to set the threshold in papaya using info from the nidm results graph? would that be using a nidm:threshold http://nidm.nidash.org/specs/nidm-results_120.html#section-nidm:'Threshold' ?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/vsoch/nidmviewer/issues/8#issuecomment-222024619, or mute the thread https://github.com/notifications/unsubscribe/AAOkp0CRqDDXiG3HlFO03cHn0LpyV5zqks5qFi7dgaJpZM4IoCtT .

cmaumet commented 8 years ago

The excursion set maps contains the statistic values of the statistic used by the thresholding (i.e. z-stat for FSL, t/f-stat for SPM).

vsoch commented 8 years ago

@chrisfilo take a look at code in master branch now (not yet pushed to pip). If there is an empty excursion set, regardless of coordinates in the table, the following is shown:

image

You did not outline this use case in your examples (eg, the case when the image is empty but there are coordinates still) and so I assumed we would want to do equivalent to (case when image is empty and there are no coordinates). If there are no peaks (but an image), the table code is modified to tell this to the user:

image

I only had one nidm.ttl to test so I was limited in testing these functions - if you want to link to other ttl files and verify what should be the outcome I'd be glad to test! If you want to install and test locally just clone, install, and go.

  git clone http://www.github.com/vsoch/nidm-viewer
  cd nidm-viewer
  sudo python setup.py sdist
  sudo python setup.py install
  cd examples
  nidmviewer fsl/nidm.ttl

Either way, when the functionality is what is desired I will update package on pip, and please let me know if there are any bugs I missed.

vsoch commented 8 years ago

I am anticipating a bug but will wait to fix it. On this line https://github.com/vsoch/nidmviewer/blob/96d1690c6368099e52ef986cd868ff42d415c95d/nidmviewer/convert.py#L31 I left in the part that @cmaumet added to add Nones to the dataframe, however this should ideally not add anything. The reason is that the data object must be empty when we render in the interface to display the "No suprathreshold voxels" as is now, likely the None objects will either lead to an error / not render. If someone has a ttl file that fits this description I can properly test - otherwise my (anticipated suggestion) is to return this line to how it was before.

cmaumet commented 8 years ago

You did not outline this use case in your examples (eg, the case when the image is empty but there are coordinates still)

Actually, this should not happen. One issue that we have currently is that the viewer does not handle properly multiple contrasts and this is why we sometime have a peak that correspond to no cluster (as you noted in https://github.com/vsoch/nidmviewer/pull/13#discussion_r65081441).

The example in examples/fsl/nidm.ttl has two excursion sets ExcursionSet_T001.nii.gz and ExcursionSet_T002.nii.gz but only ExcursionSet_T001.nii.gz is displayed by the viewer.

Could we update the viewer to

Let me know what I can do to help!

vsoch commented 8 years ago

Both excursion sets should be displayed - if you click on "brain" in the top left you can choose between the maps.

cmaumet commented 8 years ago

@vsoch: the example where the None were needed was http://neurovault.org/collections/1404/fsl_ds005_sub-01.nidm. If your update works fine with this example, feel free to remove the line in question.

cmaumet commented 8 years ago

Thanks @vsoch: I had missed the brain icon. I need to check what is wrong with those coordinates. Let me have a look and I will get back to you.

vsoch commented 8 years ago

Thanks! As i suspected, the current change adding "None" leads to a bug:

image

vsoch commented 8 years ago

No worries, I just did some fixes and tested on both nidm images and it works as should, changes are pushed. Going to bed - I'll let @chrisfilo check this out when he has a chance. G'nite! :O)

chrisgorgo commented 8 years ago

I've tried running most recent nidmviewer with NeuroVault master and I got the following error:

`Environment:

Request Method: GET Request URL: http://local.docker/collections/3/spm_example.nidm

Django Version: 1.8.8 Python Version: 2.7.11 Installed Applications: ('django.contrib.auth', 'django.contrib.contenttypes', 'django.contrib.sessions', 'django.contrib.sites', 'django.contrib.messages', 'django.contrib.staticfiles', 'neurovault.apps.main', 'neurovault.apps.statmaps', 'neurovault.apps.users', 'django.contrib.sitemaps', 'django.contrib.admin', 'social.apps.django_app.default', 'rest_framework', 'taggit', 'crispy_forms', 'coffeescript', 'taggit_templatetags', 'dbbackup', 'polymorphic', 'djcelery', 'django_cleanup', 'file_resubmit', 'django_mailgun', 'django_hstore', 'guardian', 'oauth2_provider', 'fixture_media') Installed Middleware: ('django.middleware.common.CommonMiddleware', 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.csrf.CsrfViewMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'neurovault.apps.statmaps.middleware.CollectionRedirectMiddleware')

Traceback: File "/usr/local/lib/python2.7/site-packages/django/core/handlers/base.py" in get_response

  1. response = wrapped_callback(request, _callback_args, *_callback_kwargs) File "./neurovault/apps/statmaps/views.py" in view_nidm_results
  2. button_text=button_text) File "/usr/local/lib/python2.7/site-packages/nidmviewer/viewer.py" in generate
  3. template = add_string("[SUB_EMPTY_SUB]",str(empty_images),template)

Exception Type: UnboundLocalError at /collections/3/spm_example.nidm Exception Value: local variable 'empty_images' referenced before assignment`

collections/3/spm_example.nidm is one of the NIDM images included in the fixture loaded for dev purposes when an empty NeuroVault instance is brought up. All you need to do to replicate this is to checkout current master of NeuroVault, change the Dockerfile to point to desired version of nidmviewer, rebuild the image and bring up the instance.

vsoch commented 8 years ago

ah! I forgot to test when view_in_browser = False, I'll give this a go and let you know.

vsoch commented 8 years ago

ok, fix is done, I'll test on NeuroVault tomorrow!

cmaumet commented 8 years ago

@vsoch: regarding the "empty peak issue", we have identified a bug in the nidmfsl exporter with @AlexBowring that was affecting exports with multiple contrasts (cf. https://github.com/incf-nidash/nidmresults-fsl/pull/88). New version nidmfsl 0.3.5 is now available on pypi.

It's great to be able to use the nidmviewer to test the validity of our exports. Thanks for all your work on this! We are going to do a few more tests and let you know if we identify more bugs.

vsoch commented 8 years ago

There is a new issue with regard to using the nidm object to obtain the file name AND check for empty voxels - the url will work fine once in the web interface, but given that it's used on the server side to check for empty voxels, nibabel spits out an error when the url is given to load:

image

It would be ideal if we didn't have to do this check, perhaps due to the error you mentioned above? Please let me know.

vsoch commented 8 years ago

ok, I think I could get this to work if the url was used to download the image to a temporary directory. I tested the retrieve = True, and it's not going to be testable using 127.0.0.1 because I get a connected refused:

  --2016-06-02 20:53:51--  http://127.0.0.1/collections/5/fsl_ds005_sub-01.nidm/ExcursionSet.nii.gz
  Connecting to 127.0.0.1:80... failed: Connection refused.
  /tmp/tmpJi5qhq/JWNQBB.nii.gz: Scheme missing.
  1024

The other option is to figure out how to get the actual image path represented instead of the url, but then the interface would break. It would of course be easiest to just specify these variables and not have to use nidm... :) Or remove needing to read in the image to check for empty voxels from generating the viewer.

cmaumet commented 8 years ago

@vsoch: we could definitely add an attribute in the nidm document to say whether or not the excursion set is empty. I will try and include this in our next release (cf. https://github.com/incf-nidash/nidm/issues/385). In the meantime, we might want to skip this check unless you find another solution...

chrisgorgo commented 8 years ago

@cmaumet you mentioned that "empty peak issue" was a bug. Does this mean that the example nidm files with no peak coordinates but with a non empty excursion set map you gave me were invalid and we should not expect to see such files in the wild? If that's the case we can skip this check.