Adding problematic slice_timing update

joewiner commented 7 years ago

I've cleaned up the slice timing code, but I'm getting an error here where the scipy interpolate function says "failed in converting 2nd argument `y' of dfitpack.fpcurf0 to C/Fortran array" - I'm guessing this has to do with the type of data being passed into the function? Can someone help out with this?

jdtheiss commented 7 years ago

Maybe it's related to the ellipsis on line 228 in the y numpy array.

joewiner commented 7 years ago

What do you mean?

jdtheiss commented 7 years ago

I think one of the values got cut off where it shows "...598247".

y = array([ 4.98692701, 5.17466505, 4.02527958, 5.25289685, 6.07630922, 4.41108788, 2.3280561 , 5.53387422, ...598247, 6.30956467, 5.23387126, 4.74728322, 4.10174256, 5.05153159, 5.66132716, 6.95420657, 5.84020852])

joewiner commented 7 years ago

Where is this coming from? What y array?

joewiner commented 7 years ago

Oh ok I see what you're referring to in Travis. I'm still not sure what's leading to that problem but I'll try to fix it.

jdtheiss commented 7 years ago

Never mind, I think that's just a red herring. I'm not sure what the issue is. Also unrelated, you have a comma after the "]" for time_offsets at line 29 of the slice_time_corr code.

joewiner commented 7 years ago

@matthew-brett any tips on this Fortran array conversion error?

jdtheiss commented 7 years ago

Would getting a negative value in the random data Y (from your test) cause an error?

joewiner commented 7 years ago

I don't think that is the problem, since it also failed when I tried it with a real file from the dataset

jdtheiss commented 7 years ago

Hmm, I'll take a look at it in person when we meet.

joewiner commented 7 years ago

Check out https://github.com/practical-neuroimaging/pna-utils/blob/master/slicetime.py - Matthew suggested using this for inspiration

jdtheiss commented 7 years ago

I'm actually going to get rid of my hrf function in favor of the spm_hrf one.

codecov-io commented 7 years ago

Current coverage is 92.05% (diff: 100%)

Merging #40 into master will decrease coverage by 3.38%

@@             master        #40   diff @@
==========================================
  Files            14         14          
  Lines           570        554    -16   
  Methods           0          0          
  Messages          0          0          
  Branches         42         42          
==========================================
- Hits            544        510    -34   
- Misses           26         44    +18   
  Partials          0          0

Powered by Codecov. Last update 6fd8d29...9dcb05a

mlnewton commented 7 years ago

Is it an okay to merge this, Joe?

joewiner commented 7 years ago

No not yet I'd like to take a look at padding first

On Tue, Dec 13, 2016 at 1:59 PM, mlnewton notifications@github.com wrote:

Is it an okay to merge this, Joe?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/psych-214-fall-2016/project-blue/pull/40#issuecomment-266875285, or mute the thread https://github.com/notifications/unsubscribe-auth/AFfeFOOkSR640f1hLU9yZa8sRNKLDiuKks5rHxU6gaJpZM4LJ0Qs .

joewiner commented 7 years ago

I've changed the code back so it will pull slice timing info from the .json file. I'm not going to be able to add the padding info in time. I have read over the code but don't understand where the "pad" data is supposed to come from. If someone wants to merge this, I don't think it will ruin everything...

jdtheiss commented 7 years ago

Since the code is based on the exercise from class, I think it should be fine the way it is currently written.

joewiner commented 7 years ago

Alright this is less of a mess than it appears - the way to fix this issue is to make the time_offsets an input, and the wrapper code will grab the times from the .json file. I'm going to fix this script and the test to reflect these changes. @jdtheiss since you're working on it, could you add the following lines to the wrapper code (...and correct me if I'm wrong)?

data_file = dir_utils.search_directory(data_folder, 'task-visualimageryfalsememory_bold.json') outputs = dir_utils.get_contents(data_file, ['SliceTiming']) time_offsets = outputs[0]

then the wrapper should call slice timing like this:

time_corrected_data = fmri_utils.slice_timing_corr.slice_timing_corr(data, TR, time_offsets)

jdtheiss commented 7 years ago

Ok, great. That is what is implemented in the wrapper code. Thanks!

psych-214-fall-2016 / project-blue

Adding problematic slice_timing update #40

Current coverage is 92.05% (diff: 100%)