Skip to content

Fix contiguous detection to properly handle F-contiguous input.#398

Merged
nouiz merged 2 commits intoTheano:masterfrom
abergeron:fix_bgemm
Apr 4, 2017
Merged

Fix contiguous detection to properly handle F-contiguous input.#398
nouiz merged 2 commits intoTheano:masterfrom
abergeron:fix_bgemm

Conversation

@abergeron
Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member

@nouiz nouiz left a comment

Choose a reason for hiding this comment

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

Also, this would request test for inputs fortran and strided order in Theano or here. Currently, this is only tested in Theano.

Comment thread src/gpuarray_array_blas.c Outdated
return err;
}

static inline int is_2d_contiguous(const GpuArray *a) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The name isn't clear to what it does. Currently, it always expect a 2d inputs. If you want to keep that name,use ndim-1 and ndim-2 for which dimensions you checks. Otherwise, rename.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It expects a 3d input and makes sure that the last two are contiguous.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm ok with a rename, but to what?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is_last_2d_contiguous and change the to use ndim -1 and ndim-2 in the code.

Comment thread tests/check_blas.c Outdated
GpuArray B;
GpuArray C;

size_t dims[3] = {2, 32, 32};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

typo, should be 2,3,3 how it can have passed ?

Comment thread tests/check_blas.c Outdated
GpuArray C;
ssize_t t;

size_t dims[3] = {2, 32, 32};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be 2,3,3, why the test passed?

@abergeron
Copy link
Copy Markdown
Member Author

I'm working on this, it's WIP.

@abergeron
Copy link
Copy Markdown
Member Author

Now it's final.

@nouiz
Copy link
Copy Markdown
Member

nouiz commented Apr 4, 2017

Don't forget to raise the version number

@abergeron
Copy link
Copy Markdown
Member Author

We should bundle this and the disk cache in 0.6.4

@nouiz nouiz merged commit 2237b40 into Theano:master Apr 4, 2017
@abergeron abergeron deleted the fix_bgemm branch August 2, 2017 21:21
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