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

TST : this is not exactly equal on some platforms. #396

Merged
merged 2 commits into from Aug 14, 2014

Conversation

Projects
None yet
4 participants
@arokem
Member

arokem commented Jul 29, 2014

This should address gh-395

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Jul 29, 2014

Any thoughts as to why these two aren't exactly the same?

@arokem

This comment has been minimized.

Member

arokem commented Jul 29, 2014

No clue. prediction and model_prediction ultimately both use
csdeconv.csd_predict for the calculation itself. That is,
model_prediction is calling a very thin wrapper of that function.

On Tue, Jul 29, 2014 at 9:50 AM, Matthew Brett notifications@github.com
wrote:

Any thoughts as to why these two aren't exactly the same?


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

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 5, 2014

Check - does anyone worry that apparently calling the same function twice gives a slightly different answer?

@arokem

This comment has been minimized.

Member

arokem commented Aug 5, 2014

Not really - seems to be somewhere just beyond the 7th decimal. Are you
worried?

On Tue, Aug 5, 2014 at 11:11 AM, Matthew Brett notifications@github.com
wrote:

Check - does anyone worry that apparently calling the same function twice
gives a slightly different answer?


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

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 5, 2014

I always worry when I don't understand an error :)

@MrBago

This comment has been minimized.

Contributor

MrBago commented Aug 5, 2014

I think the difference is in how R is computed, which platforms does this
fail on?

Bago

On Tue, Aug 5, 2014 at 11:36 AM, Matthew Brett notifications@github.com
wrote:

I always worry when I don't understand an error :)


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

@matthew-brett

This comment has been minimized.

Member

matthew-brett commented Aug 5, 2014

Just 32 bit linux.

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 14, 2014

So? Should we merge this? Or things are not clear yet?

@arokem

This comment has been minimized.

Member

arokem commented Aug 14, 2014

I think we can merge this - my interpretation is that there's apparently a
numerical difference way down past the 7th decimal that has to do with how
R is calculated.

On Thu, Aug 14, 2014 at 12:46 PM, Eleftherios Garyfallidis <
notifications@github.com> wrote:

So? Should we merge this? Or things are not clear yet?


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

@Garyfallidis

This comment has been minimized.

Member

Garyfallidis commented Aug 14, 2014

Okay, makes sense.

Garyfallidis added a commit that referenced this pull request Aug 14, 2014

Merge pull request #396 from arokem/fix-precision-csd-predict
TST : this is not exactly equal on some platforms.

@Garyfallidis Garyfallidis merged commit c66b0f9 into nipy:master Aug 14, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment