-
Notifications
You must be signed in to change notification settings - Fork 437
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
trilinear_interpolate4d only works on float64 #832
Conversation
So basically trying to feed float32 shm coeff does not work as per this traceback Traceback (most recent call last):
File "/home/samuel/git/scilpy/scripts/tracking.py", line 32, in <module>
save_trk(out_track, tracker, affine, seed_mask.shape)
File "/home/samuel/python/dipy/io/trackvis.py", line 31, in save_trk
nib.trackvis.write(filename, data, hdr)
File "/home/samuel/.local/lib/python2.7/site-packages/nibabel/trackvis.py", line 337, in write
streams0 = next(stream_iter)
File "/home/samuel/python/dipy/io/trackvis.py", line 24, in <genexpr>
data = ((p, None, None) for p in points)
File "/home/samuel/python/dipy/tracking/utils.py", line 916, in move_streamlines
for sl in streamlines:
File "/home/samuel/python/dipy/tracking/utils.py", line 916, in move_streamlines
for sl in streamlines:
File "/home/samuel/python/dipy/tracking/local/localtracking.py", line 108, in _generate_streamlines
directions = dg.initial_direction(s)
File "/home/samuel/python/dipy/direction/probabilistic_direction_getter.py", line 216, in initial_direction
pmf = self.pmf_gen.get_pmf(point)
File "/home/samuel/python/dipy/direction/probabilistic_direction_getter.py", line 46, in get_pmf
coeff = trilinear_interpolate4d(self.shcoeff, point)
File "dipy/tracking/local/interpolation.pyx", line 72, in dipy.tracking.local.interpolation.trilinear_interpolate4d (dipy/tracking/local/interpolation.c:2389)
ValueError: Buffer dtype mismatch, expected 'double' but got 'float' |
Could you please provide a test that fails with the old behavior? |
I did, look at the first commit, the second one is the fix. 2016-01-11 16:43 GMT+01:00 Ariel Rokem notifications@github.com:
|
Or rather I made a test fail on purpose XD |
Yes, I see that you did there, but this is not persistent in the code base. I would like you to add a test case that would prevent this behavior from creeping back into the code-base. That is, write a unit test in test_probabilistic_direction_getter that fails in the code as it is currently on |
I read that wrong the first time then, I'll try something a bit later. |
Hi @samuelstjean - could you please add a test case, instead of changing the existing test-case? That is, add an additional test case, that demonstrates the bug that this is supposed to fix. Ideally (say, if you decided to start over on another branch), you would do this in two consecutive commits, allowing us to see what the problem is (what the test is, and what failure it produces on Travis), and how you propose to fix it. |
That was supposed to be the bug and the fix, but pushing both did not I guess I can just copy paste the whole thing, but do people try different
|
So I expanded the test to run on different dtype of the same data instead, does its purpose without writing the same thing twice. |
Yep. That's was the direction of what I was looking for. Just a bit more (see Travis errors) |
I have been trying to git revert, reset --hard and a bunch of thing, will 2016-01-18 16:24 GMT+01:00 Ariel Rokem notifications@github.com:
|
This might be one for an interactive rebase On Mon, Jan 18, 2016 at 7:28 AM, Samuel St-Jean notifications@github.com
|
Any progress here? I think you might consider keeping the tests you have here, but using this approach instead of the cast to float: |
Closing in favor of #1784 |
So now it force the shm coefficient to float64. Could also patch the cython file to accept more datatype, thought maybe float32 is not precise enough, who knows.