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

BF - callable response does not work #314

Merged
merged 1 commit into from Jan 14, 2014

Conversation

Projects
None yet
3 participants
@MrBago
Contributor

MrBago commented Jan 14, 2014

This has been brought up in #313 and #218. This PR fixes the documentation (and removes a few lines of superfluous code) so we don't confuse our users.

We should also add some way of using an empirical response function with the CSD Model. I still like the API suggested in #218, but this would mean breaking backwards comparability with the last release.

BF - callable response does not work and is not tested, taking it out
for now so that it can be added back with tests and an example
@arokem

This comment has been minimized.

Member

arokem commented Jan 14, 2014

Agreed. If someone wants this kind of functionality in the future, this will have to come back with a test!

@arokem

This comment has been minimized.

Member

arokem commented Jan 14, 2014

Just waiting for the travis-bot to be happy with this.

arokem added a commit that referenced this pull request Jan 14, 2014

Merge pull request #314 from MrBago/bf_remove_callable_response
BF - callable response does not work

@arokem arokem merged commit c61589d into nipy:master Jan 14, 2014

1 check passed

default The Travis CI build passed
Details

@MrBago MrBago deleted the MrBago:bf_remove_callable_response branch Jan 14, 2014

should return an ndarray with the all the signal values for the response function. The response
function will be used as deconvolution kernel ([1]_)
response : tuple
A tuple with two elements. The first is the eigen-values as an (3,)

This comment has been minimized.

@stefanv

stefanv Jan 14, 2014

Contributor

Which eigen-values are these? And where should the signal be evaluated to give the response function?

This comment has been minimized.

@arokem

arokem Jan 14, 2014

Member

Is this a comment about the docstring, or about the procedure used?

On Tuesday, January 14, 2014, Stefan van der Walt wrote:

In dipy/reconst/csdeconv.py:

@@ -35,12 +35,12 @@ def init(self, gtab, response, reg_sphere=None, sh_order=8, lambda_=1, tau=0
Parameters
----------
gtab : GradientTable

  •    response : tuple or callable
    
  •        If tuple, then it should have two elements. The first is the eigen-values as an (3,) ndarray
    
  •        and the second is the signal value for the response function without diffusion weighting.
    
  •        This is to be able to generate a single fiber synthetic signal. If callable then the function
    
  •        should return an ndarray with the all the signal values for the response function. The response
    
  •        function will be used as deconvolution kernel ([1]_)
    
  •    response : tuple
    
  •        A tuple with two elements. The first is the eigen-values as an (3,)
    

Which eigen-values are these? And where should the signal be evaluated to
give the response function?


Reply to this email directly or view it on GitHubhttps://github.com//pull/314/files#r8861319
.

This comment has been minimized.

@stefanv

stefanv Jan 14, 2014

Contributor

It comes from the perspective of someone who doesn't know the functionality, but who would like to try to use this sometime (but wouldn't know how). I.e., I think the docstring can be clearer.

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