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

Inconsistent output for values_from_volume #909

Closed
MarcCote opened this Issue Feb 12, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@MarcCote
Contributor

MarcCote commented Feb 12, 2016

@arokem thanks for that function, really useful. I played a bit more with it and found some corner cases, that haven't been tested. For instance,

from dipy.tracking.streamline import values_from_volume
data3D = np.ones((2,2,2))
streamlines = np.ones((10, 2, 3))
values_from_volume(data3D, streamlines).shape
>>> (10, 2)  # So far so good.
streamlines = np.ones((10, 1, 3))
values_from_volume(data3D, streamlines).shape
>>> (10,)  # I would expect (10, 1)

data4D = np.ones((2,2,2,2))
streamlines = np.ones((10, 1, 3))
values_from_volume(data4D, streamlines).shape  # Even worst
>>> ValueError: bad axis1 argument to swapaxes
/data/research/src/dipy/dipy/tracking/streamline.py in values_from_volume(data, streamlines, affine)
    449 
    450         if isinstance(vals[-1], np.ndarray):
--> 451             return np.swapaxes(np.array(vals), 2, 1).T
    452         else:
    453             new_vals = []

/usr/lib/python2.7/dist-packages/numpy/core/fromnumeric.pyc in swapaxes(a, axis1, axis2)
    487     except AttributeError:
    488         return _wrapit(a, 'swapaxes', axis1, axis2)
--> 489     return swapaxes(axis1, axis2)
    490 
    491 

ValueError: bad axis1 argument to swapaxes

I think I found the source of the problem; the .squeeze() here:
https://github.com/nipy/dipy/blob/master/dipy/tracking/streamline.py#L394

Is the squeeze really necessary?

arokem added a commit to arokem/dipy that referenced this issue Feb 12, 2016

@arokem

This comment has been minimized.

Member

arokem commented Feb 12, 2016

Thanks for reporting this. As you can see I've started a PR to work on this.

Just to understand how this arises: is this an actual use-case? Do you actually have a situation where you have a bunch of streamlines and each one has just one node? That might be a tractography problem :-)

@arokem

This comment has been minimized.

Member

arokem commented Feb 12, 2016

And to answer your question: that squeeze seems to be necessary for some other use-cases. You'll see the test failure appear soon for this commit: arokem@a0eb5be

@arokem

This comment has been minimized.

Member

arokem commented Feb 12, 2016

Is there something obviously wrong about the fix in the most recent commit on #910?

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Feb 12, 2016

Yes, it is an actual use-case and no, it is not a tractography problem. On the contrary, it was arising during the tracking ;-). I'm extracting values from a volume one point at the time for many streamlines in parallel.

@MarcCote

This comment has been minimized.

Contributor

MarcCote commented Apr 21, 2017

It seems I forgot to close this one, so I'm closing it now.

@MarcCote MarcCote closed this Apr 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment