[Nipy-devel] [dipy] request for code review

Bago mrbago@gmail....
Mon Aug 1 18:13:58 CDT 2011


Ya I suspected that we would need to do that eventually, and fact-tracking
should be renamed because the stuff in there is not strictly fact anymore.
At some point I'll take a look how to best organize it. There are also a few
things I didn't include in the branch which I should put somewhere at some
point, like electrostatic repulsion of points on a sphere.

I also have a few ideas on how we might do what you're describing. I'm not
in any rush, but when we both have time we should either email or talk over
skype about the api and what we want it to look like. I want to keep some
method in the api with special meaning (ie fit_data, evaluate, next_step and
propagate), but that doesn't mean we can't add other methods/functions.

I'm glad you liked my code
Bago

On Mon, Aug 1, 2011 at 2:53 AM, Eleftherios Garyfallidis <
garyfallidis@gmail.com> wrote:

> Hi Bago,
>
> You have many nice goodies in your code and the code is in general very
> well written. We will need to change some things mainly on the structure of
> the code so we can do the merge to the main repos. For example there are too
> many things inside spherical_harmonic_models.py which need to go to
> different files I think.
>
> Also in your reconstruction classes you are using a very different way to
> call them than what we use with GQI or DSI. Your write for example
>
>     model_instance = MonoExpOpdfModel(sh_order, bval, bvec, .006)
>     model_instance.set_sampling_points(verts, edges)
>     opdfs_sampled_at_verts = model_instance.evaluate(norm_data)
>     opdfs_sph_harm_coef = model_instance.fit_data(norm_data)
>
> I would prefer something like
>
>    me= MonoExpOpdf(data,bvals,gradients, data, bvals, gradients,param=.006,
> odf_sphere=(verts,edges), mask=None,auto=True)
>
> If auto is True then the constructor is calling directly the fit parts.
> Then I would use something like
>
>   me.sh_coeffs() to get the coefficients.
>
> I think it will be trivial to do any necessary changes as your code is well
> writen. Give me a few more days to think how we would be able to do this. I
> need to send a chapter for corrections to my supervisors and I am a bit on
> the run at the moment.
>
> GGT,
> Eleftherios
>
>
> On Thu, Jul 28, 2011 at 11:19 PM, Bago <mrbago@gmail.com> wrote:
>
>> Sorry, bootspeed.pyx was a scratch file I had used to test some stuff. It
>> didn't have anything useful so I never committed it. I took out the
>> reference to it from setup.py so it should work now.
>>
>> Bago
>>
>>
>> On Thu, Jul 28, 2011 at 2:59 PM, Eleftherios Garyfallidis <
>> garyfallidis@gmail.com> wrote:
>>
>>> Hi Bago,
>>>
>>> I am trying to compile your spherical_harmonic_models branch and getting
>>> this error:
>>>
>>> Traceback (most recent call last):
>>>   File "setup.py", line 96, in <module>
>>>     extbuilder = cyproc_exts(EXTS, CYTHON_MIN_VERSION, 'pyx-stamps')
>>>   File "/home/eg309/Devel/dipy/cythexts.py", line 98, in cyproc_exts
>>>     if stamped_pyx_ok(exts, hash_stamps_fname):
>>>   File "/home/eg309/Devel/dipy/cythexts.py", line 43, in stamped_pyx_ok
>>>     source_hash = sha1(open(source, 'rt').read()).hexdigest()
>>> IOError: [Errno 2] No such file or directory:
>>> 'dipy/tracking/bootspeed.pyx'
>>> make: *** [distances.so] Error 1
>>>
>>> Is there a file missing that I need to worry about? Or should I ignore
>>> this for now?
>>>
>>> I called the spherical_harmonic_vis_example.py and it does work however
>>> mayavi is getting too slow when I try to visualize it and I can not see
>>> properly the result even with a couple of voxels.
>>> Will try it again tomorrow on a machine with a better graphics card.
>>>
>>> GGT,
>>> Eleftherios
>>>
>>>
>>>
>>>
>>> On Thu, Jul 28, 2011 at 1:05 PM, Eleftherios Garyfallidis <
>>> garyfallidis@gmail.com> wrote:
>>>
>>>>
>>>> Thank you Bago. Will have a careful look and get back to you. :-)
>>>>
>>>>
>>>> On Thu, Jul 28, 2011 at 2:32 AM, Matthew Brett <matthew.brett@gmail.com
>>>> > wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Wed, Jul 27, 2011 at 5:43 PM, Bago <mrbago@gmail.com> wrote:
>>>>> > Hi all,
>>>>> >   I've written some code for dipy and even though I've been working
>>>>> to
>>>>> > improve it I think it's time to get some feedback. The changes I've
>>>>> made are
>>>>> > in the spherical_harmonic_models branch of my fork of the dipy repo
>>>>> on
>>>>> > github. I've written two examples in the examples directory, and most
>>>>> of the
>>>>> > code I've written is in dipy.reconst.spherical_harmonic_models and
>>>>> > dipy.tracking.fact_tracking. The later file will probably need to be
>>>>> renamed
>>>>> > and it doesn't have tests yet but I've done my best to document it.
>>>>> >
>>>>> > I don't want to say too much more because I'm hoping the examples and
>>>>> > documentation are enough to explain it, but let me know if there are
>>>>> any
>>>>> > question. Thanks in advance for you feedback, and I hope we can merge
>>>>> it
>>>>> > into the main branch as soon a few people have looked at it.
>>>>>
>>>>> This:
>>>>>
>>>>>
>>>>> https://github.com/MrBago/dipy/compare/Garyfallidis:master...MrBago:spherical_harmonic_models
>>>>>
>>>>> gives a nice compare view,
>>>>>
>>>>> See you,
>>>>>
>>>>> Matthew
>>>>> _______________________________________________
>>>>> Nipy-devel mailing list
>>>>> Nipy-devel@neuroimaging.scipy.org
>>>>> http://mail.scipy.org/mailman/listinfo/nipy-devel
>>>>>
>>>>
>>>>
>>>
>>> _______________________________________________
>>> Nipy-devel mailing list
>>> Nipy-devel@neuroimaging.scipy.org
>>> http://mail.scipy.org/mailman/listinfo/nipy-devel
>>>
>>>
>>
>> _______________________________________________
>> Nipy-devel mailing list
>> Nipy-devel@neuroimaging.scipy.org
>> http://mail.scipy.org/mailman/listinfo/nipy-devel
>>
>>
>
> _______________________________________________
> Nipy-devel mailing list
> Nipy-devel@neuroimaging.scipy.org
> http://mail.scipy.org/mailman/listinfo/nipy-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.scipy.org/pipermail/nipy-devel/attachments/20110801/e3544c3a/attachment.html 


More information about the Nipy-devel mailing list