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

Sphere and default used through the code #312

Closed
samuelstjean opened this Issue Jan 13, 2014 · 6 comments

Comments

Projects
None yet
5 participants
@samuelstjean
Contributor

samuelstjean commented Jan 13, 2014

Sometimes, the code uses a 362 points or a 724 points sphere by default. It seems strange to not use a fixed default, since some parts of the estimation might be imprecise because of that.

For example, reconst/csdeconv.py uses a 362 points sphere on line 89 if nothing is supplied, but the peaks_from_model uses a 724 points sphere in the examples.

Also, the doc for EuDX is not consistent. In tracking/eudx.py on line 83, the doc says it uses a 362 point sphere but on line 155 it actually uses a 724 points sphere.

@arokem

This comment has been minimized.

Member

arokem commented Jan 13, 2014

The EuDX documentation thing should be considered a bug.

For all the rest, I would prefer to always go with the 362 point sphere,
and let users set it to 724 if they want. I think that it's OK to do use
724 in the examples, just to show how it's done, but maybe the examples
would actually build faster if we used 362 everywhere?

On Mon, Jan 13, 2014 at 1:29 PM, Samuel St-Jean notifications@github.comwrote:

Sometimes, the code uses a 362 points or a 724 points sphere by default.
It seems strange to not use a fixed default, since some parts of the
estimation might be imprecise because of that.

For example, reconst/csdeconv.py uses a 362 points sphere on line 89 if
nothing is supplied, but the peaks_from_model uses a 724 points sphere in
the examples.

Also, the doc for EuDX is not consistent. In tracking/eudx.py on line 83,
the doc says it uses a 362 point sphere but on line 155 it actually uses a
724 points sphere.


Reply to this email directly or view it on GitHubhttps://github.com//issues/312
.

@MrBago

This comment has been minimized.

Contributor

MrBago commented Jan 13, 2014

How about using a HemiSphere with 362 points as the default sphere, all the nutrition of a 724 Sphere with only half the calories :). No but seriously?

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 14, 2014

Let's do the most important thing first let's remove the default option for the sphere from the parameters. That should resolve the problem that Sam found. I will make a PR.

@stefanv

This comment has been minimized.

Contributor

stefanv commented Jan 14, 2014

+1 on the Hemisphere

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Jan 14, 2014

To answer to @arokem's suggestion. EuDX's speed is not affected by the size of the sphere. In theory it should also work with a Hemisphere but I haven't tried it. For other problems for example in CSD the size of the sphere is important in the tests especially if you are validating angular resolution in the test.

The examples should have good quality and some of the quality is related to the resolution of the sphere. I would strongly disagree on reducing the quality of the sphere in the examples to make them run faster. People might start complaining about the quality of their results when they only need to use a better sphere.

Actually, there is no time issue anymore with the examples as you can select to run only those that you need to validate and unselect the others. It is only when building the website that you might need to run them all and that shouldn't take more than 30' in a standard PC.

@arokem

This comment has been minimized.

Member

arokem commented Jan 14, 2014

OK.

On Tue, Jan 14, 2014 at 12:33 PM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

To answer to @arokem https://github.com/arokem's suggestion. EuDX's
speed is not affected by the size of the sphere. In theory it should also
work with a Hemisphere but I haven't tried it. For other problems for
example in CSD the size of the sphere is important in the tests especially
if you are validating angular resolution in the test.

The examples should have good quality and some of the quality is related
to the resolution of the sphere. I would strongly disagree on reducing the
quality of the sphere in the examples to make them run faster. People might
start complaining about the quality of their results when they only need to
use a better sphere.

Actually, there is no time issue anymore with the examples as you can
select to run only those that you need to validate and unselect the others.
It is only when building the website that you might need to run them all
and that shouldn't take more than 30' in a standard PC.


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

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