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

DiffusionSpectrumModel assumes 1 b0 and fails with data with more than 1 b0 #153

Closed
MrBago opened this Issue Mar 28, 2013 · 7 comments

Comments

Projects
None yet
3 participants
@MrBago
Contributor

MrBago commented Mar 28, 2013

I started to fix this and I can share what I have so far with anyone willing to help me. In the mean time here is a small script to re-create the issue, https://gist.github.com/MrBago/5260154

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Mar 28, 2013

Hi Bago. You probably already know this. DiffusionSpectrumModel will not work with spherical gradients as those of the Stanford dataset. You can use the Taiwan dataset and add some extra S0s on top of that. Additionally, in theory DSI expects only one S0. We could have an automated strategy to average all S0s if they are more than one. Or perhaps we can at least raise an error if they are more than one? I think currently it should still work with more than one and it should only use the last one. Thank you for looking at this.

@arokem

This comment has been minimized.

Member

arokem commented Jan 11, 2014

Is this still an issue? @Garyfallidis? @MrBago? Should we close it?

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 14, 2014

I think it is still an issue.

@arokem

This comment has been minimized.

Member

arokem commented Jan 14, 2014

OK - let's leave it here for now (and possibly try to fix it some time ...
:-D)

On Mon, Jan 13, 2014 at 5:33 PM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

I think it is still an issue.


Reply to this email directly or view it on GitHubhttps://github.com//issues/153#issuecomment-32231155
.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 14, 2014

Okay, we need to think of this. DSI by the way it is constructed requires a very specific set of gradient directions and one b0. This is what the theory says. So, if you start giving it data which are not DSI data you have to tell it what to do with them but this belongs to the preprocessing step in my view. So, the user has to prepare the data before starting the DSI reconstrucion himself. A similar problem takes place with half sphere dsi or full sphere dsi. What do you do then? I think the best solution would be to have a check to see if the acquisition is a proper DSI acquisition (using gtab) and through an error if it is not. How does that sound @MrBago, @arokem?

@arokem

This comment has been minimized.

Member

arokem commented Jan 14, 2014

Sounds good to me. So - you would check if there is more than one b0 and
throw an error for that? Would you also check that the bvecs/bvals form a
grid in q space? That might be good.

On Mon, Jan 13, 2014 at 5:50 PM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

Okay, we need to think of this. DSI by the way it is constructed requires
a very specific set of gradient directions and one b0. This is what the
theory says. So, if you start giving it data which are not DSI data you
have to tell it what to do with them but this belongs to the preprocessing
step in my view. So, the user has to prepare the data before starting the
DSI reconstrucion himself. A similar problem takes place with half sphere
dsi or full sphere dsi. What do you do then? I think the best solution
would be to have a check to see if the acquisition is a proper DSI
acquisition (using gtab) and through an error if it is not. How does that
sound @MrBago https://github.com/MrBago, @arokemhttps://github.com/arokem
?


Reply to this email directly or view it on GitHubhttps://github.com//issues/153#issuecomment-32232005
.

@arokem

This comment has been minimized.

Member

arokem commented Jan 27, 2016

This was resolved at long last with #753. Closing.

@arokem arokem closed this Jan 27, 2016

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