mrtrix 0.3 default basis is different from mrtrix 0.2

[samuelstjean]

So according to this https://github.com/jdtournier/mrtrix3/wiki/Orthonormal-SH-basis, the new mrtrix 0.3 will have an orthornormal basis. That means that any function using the -basis option will either

1. Need to call it mrtrix 0.2 basis to distinguish the old from the new one.
2. Implement a mrtrix 0.2 and mrtrix 0.3 basis since they are not the same.

They now say it will be orthornormal, so if it fits with dipy basis all is good, else the new basis will have to be implemented when the 0.3 release is more mainstream.

[samuelstjean]

I just ran a test to check something, and I think dipy will have to support another basis. Like the webpage explains, odf looks fine, but they are still screwed nevertheless computation wise.

[samuelstjean]

Looking at https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of sqrt(2). Anyone else willing to double check if that's correct? According to my previous post, it's it's only for switching to an orthonromal basis ,then it's really just a scale factor like the script seems to do.

[mdesco]

This is correct. It is a factor sqrt(2). If MRtrix has done the switch to a
normalize basis, we should do it too. Since CSD and mrtrix is not the
default, maybe we can just do it without worrying about backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean notifications@github.com
wrote:

Looking at
https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of sqrt(2). Anyone else
willing to double check if that's correct? According to my previous post,
it's it's only for switching to an orthonromal basis ,then it's really just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:
Web Bug from
https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif]

[arokem]

I don't think we should worry about backwards compatibility for this
particular detail. I doubt that there are many users outside of SCIL that
rely on the particular formulation of the basis set for this model. Better
to be directly compatible with mrtrix3, than with mrtrix2, I think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has done the switch to
a
normalize basis, we should do it too. Since CSD and mrtrix is not the
default, maybe we can just do it without worrying about backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of sqrt(2). Anyone
else
willing to double check if that's correct? According to my previous
post,
it's it's only for switching to an orthonromal basis ,then it's really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub
https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:
Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif]

—
Reply to this email directly or view it on GitHub
#392 (comment).

[samuelstjean]

Or instead both could be supported for now, and deprecated later on to
not break compatibility. It's really just applying a sqrt2 factor to all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility for this
particular detail. I doubt that there are many users outside of SCIL that
rely on the particular formulation of the basis set for this model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has done the
switch to
a
normalize basis, we should do it too. Since CSD and mrtrix is not the
default, maybe we can just do it without worrying about backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of sqrt(2). Anyone
else
willing to double check if that's correct? According to my previous
post,
it's it's only for switching to an orthonromal basis ,then it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:
Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif]

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[arokem]

Yes, a norm_factor kwarg set to sqrt2 per default seems appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated later on to
not break compatibility. It's really just applying a sqrt2 factor to all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility for this
particular detail. I doubt that there are many users outside of SCIL that
rely on the particular formulation of the basis set for this model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has done the
switch to
a
normalize basis, we should do it too. Since CSD and mrtrix is not the
default, maybe we can just do it without worrying about backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of sqrt(2). Anyone
else
willing to double check if that's correct? According to my previous
post,
it's it's only for switching to an orthonromal basis ,then it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:
Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif
]

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[samuelstjean]

Not necessarily by default, I would feel more confortable offering the
option to use a mrtrix3 basis, which applies the factor and thus keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old version,
anyone using an already existing script will get different odf without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated later on to
not break compatibility. It's really just applying a sqrt2 factor to
all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility for this
particular detail. I doubt that there are many users outside of
SCIL that
rely on the particular formulation of the basis set for this model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has done the
switch to
a
normalize basis, we should do it too. Since CSD and mrtrix is
not the
default, maybe we can just do it without worrying about backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of sqrt(2).
Anyone
else
willing to double check if that's correct? According to my
previous
post,
it's it's only for switching to an orthonromal basis ,then it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif
]

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[arokem]

OK - how about a norm_factor kwarg set to whatever value the
normalization factor is now (1?), and with a deprecation warning stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable offering the
option to use a mrtrix3 basis, which applies the factor and thus keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old version,
anyone using an already existing script will get different odf without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated later on to
not break compatibility. It's really just applying a sqrt2 factor to
all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility for this
particular detail. I doubt that there are many users outside of
SCIL that
rely on the particular formulation of the basis set for this model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has done the
switch to
a
normalize basis, we should do it too. Since CSD and mrtrix is
not the
default, maybe we can just do it without worrying about backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of sqrt(2).
Anyone
else
willing to double check if that's correct? According to my
previous
post,
it's it's only for switching to an orthonromal basis ,then it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[samuelstjean]

That could be an inner conversion, I would explicitely call them mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value the
normalization factor is now (1?), and with a deprecation warning stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable offering the
option to use a mrtrix3 basis, which applies the factor and thus keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old version,
anyone using an already existing script will get different odf without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated later
on to
not break compatibility. It's really just applying a sqrt2
factor to
all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility
for this
particular detail. I doubt that there are many users outside of
SCIL that
rely on the particular formulation of the basis set for this
model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has done the
switch to
a
normalize basis, we should do it too. Since CSD and mrtrix is
not the
default, maybe we can just do it without worrying about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of
sqrt(2).
Anyone
else
willing to double check if that's correct? According to my
previous
post,
it's it's only for switching to an orthonromal basis ,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[arokem]

I would prefer to write the actual number in the function signature, and
then explain in the docs why sqrt2 is a better choice (right?), and which
version of mrtrix uses which one. Would you be so kind as to make a PR with
that? Thanks!

On Thu, Dec 11, 2014 at 2:05 PM, Samuel St-Jean notifications@github.com
wrote:

That could be an inner conversion, I would explicitely call them mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value the
normalization factor is now (1?), and with a deprecation warning stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable offering the
option to use a mrtrix3 basis, which applies the factor and thus keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old version,
anyone using an already existing script will get different odf without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated later
on to
not break compatibility. It's really just applying a sqrt2
factor to
all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility
for this
particular detail. I doubt that there are many users outside of
SCIL that
rely on the particular formulation of the basis set for this
model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has done the
switch to
a
normalize basis, we should do it too. Since CSD and mrtrix is
not the
default, maybe we can just do it without worrying about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of
sqrt(2).
Anyone
else
willing to double check if that's correct? According to my
previous
post,
it's it's only for switching to an orthonromal basis ,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[samuelstjean]

Apparently nobody reads the doc, that might be too dangerous for unaware
users.
Le 2014-12-11 17:09, Ariel Rokem a écrit :

I would prefer to write the actual number in the function signature, and
then explain in the docs why sqrt2 is a better choice (right?), and which
version of mrtrix uses which one. Would you be so kind as to make a PR
with
that? Thanks!

On Thu, Dec 11, 2014 at 2:05 PM, Samuel St-Jean
notifications@github.com
wrote:

That could be an inner conversion, I would explicitely call them
mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be
clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value the
normalization factor is now (1?), and with a deprecation warning
stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable
offering the
option to use a mrtrix3 basis, which applies the factor and thus
keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old version,
anyone using an already existing script will get different odf
without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated
later
on to
not break compatibility. It's really just applying a sqrt2
factor to
all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility
for this
particular detail. I doubt that there are many users
outside of
SCIL that
rely on the particular formulation of the basis set for this
model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has
done the
switch to
a
normalize basis, we should do it too. Since CSD and
mrtrix is
not the
default, maybe we can just do it without worrying about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of
sqrt(2).
Anyone
else
willing to double check if that's correct? According
to my
previous
post,
it's it's only for switching to an orthonromal basis
,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[arokem]

OK - how about creating a PR with what you envision here, and then we can
take the discussion on the PR?

On Thu, Dec 11, 2014 at 2:15 PM, Samuel St-Jean notifications@github.com
wrote:

Apparently nobody reads the doc, that might be too dangerous for unaware
users.
Le 2014-12-11 17:09, Ariel Rokem a écrit :

I would prefer to write the actual number in the function signature, and
then explain in the docs why sqrt2 is a better choice (right?), and which
version of mrtrix uses which one. Would you be so kind as to make a PR
with
that? Thanks!

On Thu, Dec 11, 2014 at 2:05 PM, Samuel St-Jean
notifications@github.com
wrote:

That could be an inner conversion, I would explicitely call them
mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be
clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value the
normalization factor is now (1?), and with a deprecation warning
stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable
offering the
option to use a mrtrix3 basis, which applies the factor and thus
keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old version,
anyone using an already existing script will get different odf
without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated
later
on to
not break compatibility. It's really just applying a sqrt2
factor to
all
coefficients it seems, so not much code duplication in itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards compatibility
for this
particular detail. I doubt that there are many users
outside of
SCIL that
rely on the particular formulation of the basis set for this
model.
Better
to be directly compatible with mrtrix3, than with mrtrix2, I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has
done the
switch to
a
normalize basis, we should do it too. Since CSD and
mrtrix is
not the
default, maybe we can just do it without worrying about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of
sqrt(2).
Anyone
else
willing to double check if that's correct? According
to my
previous
post,
it's it's only for switching to an orthonromal basis
,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[jchoude]

If I can just chime in here, from my experience, it would be best to add
the argument with a default value that replicates the behavior of mrtrix
0.2, since people may have scripts that already run and produce those kind
of ODFs. Changing the behavior without any warning could have undesired
side-effects.

Maybe the basis of Mrtrix 3 could have a different "codename", eg: Mrtrix3.
In that case, it would make it explicit that there is a difference between
both bases.

There could be a warning that the default behavior will change in a future
release.

I think it would be nice to have a small function that converts from one
basis to the other, so that people can convert easily.

Sadly, there is no easy way to detect which basis is used by only looking
at the SH coefficients, so we cannot detect the format and issue warnings.
However, there should be a way to detect it if you also have access to the
raw data, and compare the energy profile.

As a side note, general question for people here: how do you keep
information such as this when you create pipelines? For example, let's say
you have a script that does the whole pipeline from raw data to tracts.
Let's also say that, for whatever reason, you compute fODFs and save the SH
coefficients twice, once in the Dipy basis, once in the Mrtrix basis. How
do you distinguish between both? Only keeping the info in the filename?
That seems highly impratical and error-prone. What is your usual way to do
this? Using a common image format (Nifti2, Gifti, etc) that also allows
arbitrary metadata? if so, which one?

Cheers!

2014-12-11 17:26 GMT-05:00 Ariel Rokem notifications@github.com:

OK - how about creating a PR with what you envision here, and then we can
take the discussion on the PR?

On Thu, Dec 11, 2014 at 2:15 PM, Samuel St-Jean notifications@github.com

wrote:

Apparently nobody reads the doc, that might be too dangerous for unaware
users.
Le 2014-12-11 17:09, Ariel Rokem a écrit :

I would prefer to write the actual number in the function signature,
and
then explain in the docs why sqrt2 is a better choice (right?), and
which
version of mrtrix uses which one. Would you be so kind as to make a PR
with
that? Thanks!

On Thu, Dec 11, 2014 at 2:05 PM, Samuel St-Jean
notifications@github.com
wrote:

That could be an inner conversion, I would explicitely call them
mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be
clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value the
normalization factor is now (1?), and with a deprecation warning
stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable
offering the
option to use a mrtrix3 basis, which applies the factor and thus
keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old
version,
anyone using an already existing script will get different odf
without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated
later
on to
not break compatibility. It's really just applying a sqrt2
factor to
all
coefficients it seems, so not much code duplication in
itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards
compatibility
for this
particular detail. I doubt that there are many users
outside of
SCIL that
rely on the particular formulation of the basis set for
this
model.
Better
to be directly compatible with mrtrix3, than with mrtrix2,
I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has
done the
switch to
a
normalize basis, we should do it too. Since CSD and
mrtrix is
not the
default, maybe we can just do it without worrying about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor of
sqrt(2).
Anyone
else
willing to double check if that's correct? According
to my
previous
post,
it's it's only for switching to an orthonromal basis
,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[arokem]

Since the difference between the bases is in the normalization factor, I do
think that we should encode it as such and not with a string (e.g.
'mrtrix3'). I would enthusiastically welcome having this also further
explained in the docs. Anyone want to put in a PR on this?

To your question, I have personally not really crossed the dipy-mrtrix
boundary many times. I analyze data in one system or the other, so I don't
have much experience with all the potential hybrids. I would be interested
to hear what others do about it. As is, there are indeed many opportunities
to make a variety of errors.

On Tue, Dec 16, 2014 at 10:58 AM, Jean-Christophe Houde <
notifications@github.com> wrote:

If I can just chime in here, from my experience, it would be best to add
the argument with a default value that replicates the behavior of mrtrix
0.2, since people may have scripts that already run and produce those kind
of ODFs. Changing the behavior without any warning could have undesired
side-effects.

Maybe the basis of Mrtrix 3 could have a different "codename", eg:
Mrtrix3.
In that case, it would make it explicit that there is a difference between
both bases.

There could be a warning that the default behavior will change in a future
release.

I think it would be nice to have a small function that converts from one
basis to the other, so that people can convert easily.

Sadly, there is no easy way to detect which basis is used by only looking
at the SH coefficients, so we cannot detect the format and issue warnings.
However, there should be a way to detect it if you also have access to the
raw data, and compare the energy profile.

As a side note, general question for people here: how do you keep
information such as this when you create pipelines? For example, let's say
you have a script that does the whole pipeline from raw data to tracts.
Let's also say that, for whatever reason, you compute fODFs and save the
SH
coefficients twice, once in the Dipy basis, once in the Mrtrix basis. How
do you distinguish between both? Only keeping the info in the filename?
That seems highly impratical and error-prone. What is your usual way to do
this? Using a common image format (Nifti2, Gifti, etc) that also allows
arbitrary metadata? if so, which one?

Cheers!

2014-12-11 17:26 GMT-05:00 Ariel Rokem notifications@github.com:

OK - how about creating a PR with what you envision here, and then we
can
take the discussion on the PR?

On Thu, Dec 11, 2014 at 2:15 PM, Samuel St-Jean <
notifications@github.com>

wrote:

Apparently nobody reads the doc, that might be too dangerous for
unaware
users.
Le 2014-12-11 17:09, Ariel Rokem a écrit :

I would prefer to write the actual number in the function signature,
and
then explain in the docs why sqrt2 is a better choice (right?), and
which
version of mrtrix uses which one. Would you be so kind as to make a
PR
with
that? Thanks!

On Thu, Dec 11, 2014 at 2:05 PM, Samuel St-Jean
notifications@github.com
wrote:

That could be an inner conversion, I would explicitely call them
mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be
clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value the
normalization factor is now (1?), and with a deprecation warning
stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable
offering the
option to use a mrtrix3 basis, which applies the factor and
thus
keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old
version,
anyone using an already existing script will get different odf
without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and deprecated
later
on to
not break compatibility. It's really just applying a sqrt2
factor to
all
coefficients it seems, so not much code duplication in
itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards
compatibility
for this
particular detail. I doubt that there are many users
outside of
SCIL that
rely on the particular formulation of the basis set for
this
model.
Better
to be directly compatible with mrtrix3, than with
mrtrix2,
I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If MRtrix has
done the
switch to
a
normalize basis, we should do it too. Since CSD and
mrtrix is
not the
default, maybe we can just do it without worrying
about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation factor
of
sqrt(2).
Anyone
else
willing to double check if that's correct? According
to my
previous
post,
it's it's only for switching to an orthonromal basis
,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
<
https://github.com/nipy/dipy/issues/392#issuecomment-66693497>.

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[samuelstjean]

I vote for the string way also, if any other basis change or if you want
to add a new one, it seems more logical than to patch stuff up with a
normalization factor.

As for the conversion, mrtrix3 has a small script to guess which basis
your data is probably using, which was just looking at the energy from
what I saw, and making a guess based on that.

Le 2014-12-16 16:38, Ariel Rokem a écrit :

Since the difference between the bases is in the normalization factor,
I do
think that we should encode it as such and not with a string (e.g.
'mrtrix3'). I would enthusiastically welcome having this also further
explained in the docs. Anyone want to put in a PR on this?

To your question, I have personally not really crossed the dipy-mrtrix
boundary many times. I analyze data in one system or the other, so I
don't
have much experience with all the potential hybrids. I would be
interested
to hear what others do about it. As is, there are indeed many
opportunities
to make a variety of errors.

On Tue, Dec 16, 2014 at 10:58 AM, Jean-Christophe Houde <
notifications@github.com> wrote:

If I can just chime in here, from my experience, it would be best to
add
the argument with a default value that replicates the behavior of
mrtrix
0.2, since people may have scripts that already run and produce
those kind
of ODFs. Changing the behavior without any warning could have undesired
side-effects.

Maybe the basis of Mrtrix 3 could have a different "codename", eg:
Mrtrix3.
In that case, it would make it explicit that there is a difference
between
both bases.

There could be a warning that the default behavior will change in a
future
release.

I think it would be nice to have a small function that converts from
one
basis to the other, so that people can convert easily.

Sadly, there is no easy way to detect which basis is used by only
looking
at the SH coefficients, so we cannot detect the format and issue
warnings.
However, there should be a way to detect it if you also have access
to the
raw data, and compare the energy profile.

As a side note, general question for people here: how do you keep
information such as this when you create pipelines? For example,
let's say
you have a script that does the whole pipeline from raw data to tracts.
Let's also say that, for whatever reason, you compute fODFs and save
the
SH
coefficients twice, once in the Dipy basis, once in the Mrtrix
basis. How
do you distinguish between both? Only keeping the info in the filename?
That seems highly impratical and error-prone. What is your usual way
to do
this? Using a common image format (Nifti2, Gifti, etc) that also allows
arbitrary metadata? if so, which one?

Cheers!

2014-12-11 17:26 GMT-05:00 Ariel Rokem notifications@github.com:

OK - how about creating a PR with what you envision here, and then we
can
take the discussion on the PR?

On Thu, Dec 11, 2014 at 2:15 PM, Samuel St-Jean <
notifications@github.com>

wrote:

Apparently nobody reads the doc, that might be too dangerous for
unaware
users.
Le 2014-12-11 17:09, Ariel Rokem a écrit :

I would prefer to write the actual number in the function
signature,
and
then explain in the docs why sqrt2 is a better choice
(right?), and
which
version of mrtrix uses which one. Would you be so kind as to
make a
PR
with
that? Thanks!

On Thu, Dec 11, 2014 at 2:05 PM, Samuel St-Jean
notifications@github.com
wrote:

That could be an inner conversion, I would explicitely call
them
mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be
clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value
the
normalization factor is now (1?), and with a deprecation
warning
stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable
offering the
option to use a mrtrix3 basis, which applies the factor and
thus
keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old
version,
anyone using an already existing script will get
different odf
without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and
deprecated
later
on to
not break compatibility. It's really just applying a
sqrt2
factor to
all
coefficients it seems, so not much code duplication in
itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards
compatibility
for this
particular detail. I doubt that there are many users
outside of
SCIL that
rely on the particular formulation of the basis
set for
this
model.
Better
to be directly compatible with mrtrix3, than with
mrtrix2,
I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If
MRtrix has
done the
switch to
a
normalize basis, we should do it too. Since CSD and
mrtrix is
not the
default, maybe we can just do it without worrying
about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation
factor
of
sqrt(2).
Anyone
else
willing to double check if that's correct?
According
to my
previous
post,
it's it's only for switching to an orthonromal
basis
,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
<
https://github.com/nipy/dipy/issues/392#issuecomment-66693497>.

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[arokem]

It would of course be great if someone wrote a script like that for dipy as
well!

On Tue, Dec 16, 2014 at 1:42 PM, Samuel St-Jean notifications@github.com
wrote:

I vote for the string way also, if any other basis change or if you want
to add a new one, it seems more logical than to patch stuff up with a
normalization factor.

As for the conversion, mrtrix3 has a small script to guess which basis
your data is probably using, which was just looking at the energy from
what I saw, and making a guess based on that.

Le 2014-12-16 16:38, Ariel Rokem a écrit :

Since the difference between the bases is in the normalization factor,
I do
think that we should encode it as such and not with a string (e.g.
'mrtrix3'). I would enthusiastically welcome having this also further
explained in the docs. Anyone want to put in a PR on this?

To your question, I have personally not really crossed the dipy-mrtrix
boundary many times. I analyze data in one system or the other, so I
don't
have much experience with all the potential hybrids. I would be
interested
to hear what others do about it. As is, there are indeed many
opportunities
to make a variety of errors.

On Tue, Dec 16, 2014 at 10:58 AM, Jean-Christophe Houde <
notifications@github.com> wrote:

If I can just chime in here, from my experience, it would be best to
add
the argument with a default value that replicates the behavior of
mrtrix
0.2, since people may have scripts that already run and produce
those kind
of ODFs. Changing the behavior without any warning could have undesired
side-effects.

Maybe the basis of Mrtrix 3 could have a different "codename", eg:
Mrtrix3.
In that case, it would make it explicit that there is a difference
between
both bases.

There could be a warning that the default behavior will change in a
future
release.

I think it would be nice to have a small function that converts from
one
basis to the other, so that people can convert easily.

Sadly, there is no easy way to detect which basis is used by only
looking
at the SH coefficients, so we cannot detect the format and issue
warnings.
However, there should be a way to detect it if you also have access
to the
raw data, and compare the energy profile.

As a side note, general question for people here: how do you keep
information such as this when you create pipelines? For example,
let's say
you have a script that does the whole pipeline from raw data to tracts.
Let's also say that, for whatever reason, you compute fODFs and save
the
SH
coefficients twice, once in the Dipy basis, once in the Mrtrix
basis. How
do you distinguish between both? Only keeping the info in the filename?
That seems highly impratical and error-prone. What is your usual way
to do
this? Using a common image format (Nifti2, Gifti, etc) that also allows
arbitrary metadata? if so, which one?

Cheers!

2014-12-11 17:26 GMT-05:00 Ariel Rokem notifications@github.com:

OK - how about creating a PR with what you envision here, and then we
can
take the discussion on the PR?

On Thu, Dec 11, 2014 at 2:15 PM, Samuel St-Jean <
notifications@github.com>

wrote:

Apparently nobody reads the doc, that might be too dangerous for
unaware
users.
Le 2014-12-11 17:09, Ariel Rokem a écrit :

I would prefer to write the actual number in the function
signature,
and
then explain in the docs why sqrt2 is a better choice
(right?), and
which
version of mrtrix uses which one. Would you be so kind as to
make a
PR
with
that? Thanks!

On Thu, Dec 11, 2014 at 2:05 PM, Samuel St-Jean
notifications@github.com
wrote:

That could be an inner conversion, I would explicitely call
them
mrtrix2
and mrtrix3 basis though, since anyone not aware of why the
normalization factor is 1 or sqrt2 or why it matters will be
clueless as
to which one to use and in which cases.
Le 2014-12-11 17:02, Ariel Rokem a écrit :

OK - how about a norm_factor kwarg set to whatever value
the
normalization factor is now (1?), and with a deprecation
warning
stating
that this will be switched to sqrt2 in a future release?

On Thu, Dec 11, 2014 at 1:44 PM, Samuel St-Jean
notifications@github.com
wrote:

Not necessarily by default, I would feel more confortable
offering the
option to use a mrtrix3 basis, which applies the factor and
thus
keeps
the mrtrix basis the same as the current version.
If you just switch it by default without offering the old
version,
anyone using an already existing script will get
different odf
without
even knowing the conversion was implicitly done.
Le 2014-12-11 16:38, Ariel Rokem a écrit :

Yes, a norm_factor kwarg set to sqrt2 per default seems
appropriate.

On Thu, Dec 11, 2014 at 10:39 AM, Samuel St-Jean
notifications@github.com
wrote:

Or instead both could be supported for now, and
deprecated
later
on to
not break compatibility. It's really just applying a
sqrt2
factor to
all
coefficients it seems, so not much code duplication in
itself.
Le 2014-12-11 08:52, Ariel Rokem a écrit :

I don't think we should worry about backwards
compatibility
for this
particular detail. I doubt that there are many users
outside of
SCIL that
rely on the particular formulation of the basis
set for
this
model.
Better
to be directly compatible with mrtrix3, than with
mrtrix2,
I
think.

On Mon, Dec 8, 2014 at 2:08 PM, Maxime Descoteaux
notifications@github.com
wrote:

This is correct. It is a factor sqrt(2). If
MRtrix has
done the
switch to
a
normalize basis, we should do it too. Since CSD and
mrtrix is
not the
default, maybe we can just do it without worrying
about
backward
compatibility? I let the Dipy guru's decide.

Max

Maxime Descoteaux, PhD
Professor
Sherbrooke Connectivity Imaging Laboratory (SCIL)
Centre de Recherche CHUS
Computer Science department
Sherbrooke University
2500 Boul. Université
Sherbrooke, Québec
J1K 2R1, Canada
phone: +1 819 821-8000 x66129
fax: +1 819 821-8200
http://scil.dinf.usherbrooke.ca

Scientific director
Visualization and Image Analysis Plateform (PAVI)
http://pavi.dinf.usherbrooke.ca/

On Fri, Dec 5, 2014 at 6:10 PM, Samuel St-Jean
notifications@github.com
wrote:

Looking at

https://github.com/jdtournier/mrtrix3/blob/de27b521f1807036129ac9356611516c0084cc8f/cmd/shbasis.cpp#L282

The only change seems ot be a normalisation
factor
of
sqrt(2).
Anyone
else
willing to double check if that's correct?
According
to my
previous
post,
it's it's only for switching to an orthonromal
basis
,then
it's
really
just
a scale factor like the script seems to do.

—
Reply to this email directly or view it on GitHub

https://github.com/nipy/dipy/issues/392#issuecomment-65869526.[image:

Web Bug from

https://github.com/notifications/beacon/ACEgB3FfXX36BCUW4wQKecI_6mmr14cjks5nUjLmgaJpZM4CQJS0.gif

]

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
<
https://github.com/nipy/dipy/issues/392#issuecomment-66693497>.

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub

#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

—
Reply to this email directly or view it on GitHub
#392 (comment).

[jhlegarreta]

Partially addressed by #1657 and #1658.

ToDo's:

• Explain why the sqrt(2) is relevant.
• Add a function to infer the SH basis used by the data, as it has been suggested.

[jchoude]

Just to make sure, @jhlegarreta , you say that Dipy stopped using Mrtrix0.2 for the basis?

[jhlegarreta]

True, a confusion on my side. Edited/removed the part. The factor is not actually present in the code as you said in #1657, and as shown in the new doc sh_basis.rst doc.

[scott-trinkle]

Raised a separate issue (#1782), but it looks like I should move the discussion here.

I think something is still off between the default DIPY and MRtrix3 bases. It doesn't look like the current DIPY basis_type implementations are accounting for the following quote from the dwi2fod documentation in MRtrix:

Note that the spherical harmonics equations used here differ slightly from those conventionally used, in that the (-1)^m factor has been omitted.

So currently, both software packages include the sqrt(2) scaling factor, but have opposite conventions for taking the Real vs. Imaginary components for m > 0 or m < 0 (which amounts to just reordering the way in which the SH coefficients are stored), and MRtrix does not have the (-1)^m factor included in scipy.special.sph_harm, which amounts to certain coefficients being off by a factor of -1.

Currently, the basis_type='tournier07' argument calls to the real_sym_sh_mrtrix function, which correctly reorders the coefficients, but does not include the sqrt(2) factor and does not account for the (-1)^m factor.

Here I show that when you expand the same spherical function onto SH coefficients in both DIPY and MRtrix, you can use a (admittedly sloppy for now) utility function to convert mrtrix coefficients into the dipy basis, and work with them from there. Using the basis_type='tournier07' results in different ODFs.

[skoudoro]

Hi @CHrlS98,

Since you are looking and updating the SH basis via PR #2206, Could you also look at this issue and give feedback to the community, please?

Thank you