Skip to content
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

BUG: disable BLAS for too large arrays (int overflow), closes #1189 #1190

Merged
merged 1 commit into from
Oct 19, 2017

Conversation

kohr-h
Copy link
Member

@kohr-h kohr-h commented Oct 11, 2017

No description provided.

@adler-j
Copy link
Member

adler-j commented Oct 11, 2017

Blas only works for int32? We truly need to check if this is really the case with the underlying implementation, sounds outrageous. Are we sure it is not our native call?

@adler-j
Copy link
Member

adler-j commented Oct 11, 2017

Anyway, I'd say add a largescale test for this before we merge.

@kohr-h
Copy link
Member Author

kohr-h commented Oct 11, 2017

There are weird things going on in BLAS, see also this issue. And yes, you can trigger the error in #1189 completely independently of ODL, no native issue whatsoever.

@adler-j
Copy link
Member

adler-j commented Oct 12, 2017

Now I never thought we'd find BLAS bugs by running ODL, we truly are pushing some limits here.

My suggestion is we add a largescale test for a very large space and test all arithmetic on it. I think all that is needed is basically adding another space to a fixture

@kohr-h
Copy link
Member Author

kohr-h commented Oct 12, 2017

Note that there is another issue with using BLAS for float arrays that falls into the category "wrong, but expected". The nastiness of this one is that it depends on the size of the final result as well as on the implementation of the summation itself (size of partial sums etc.).

Copy link
Member

@adler-j adler-j left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this. Long term we could do sub-sets ourselves.

@adler-j adler-j merged commit c9c5acf into odlgroup:master Oct 19, 2017
@kohr-h
Copy link
Member Author

kohr-h commented Oct 19, 2017

Approving this. Long term we could do sub-sets ourselves.

I had the same thought lately. Made a new issue: #1200

@kohr-h kohr-h deleted the issue-1189__blas_bigint branch October 19, 2017 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants