Add potrs with MAGMA #364

Merged
merged 1 commit into from Mar 22, 2016

Projects

None yet

3 participants

@bamos
Contributor
bamos commented Mar 13, 2016

Hi, this PR adds potrs with MAGMA.

I followed the structure of the existing potrf code and copied the
potrs argument checks from THTensorLapack.
The test compares the output to Torch's CPU version and is passing on my system.

This is my first time modifying cutorch and any Torch internals.
Can somebody check that I used THCudaTensor_newColumnMajor and free correctly?
I used rb_ as the source for b_ because magma_spotrs_gpu returns results in b_data.
I'm not sure if I should use th_magma_smalloc_pinned or THCudaTensor_newColumnMajor for a_data.

-Brandon.

@colesbury colesbury commented on an outdated diff Mar 21, 2016
lib/THC/THCTensorMathMagma.cu
+ int n = a->size[0];
+ int nrhs = b->size[1];
+
+ THCudaTensor *b_ = THCudaTensor_newColumnMajor(state, rb_, b);
+ float *b_data = THCudaTensor_data(state, b_);
+ THCudaTensor *a_ = THCudaTensor_newColumnMajor(state, a, a);
+ float *a_data = THCudaTensor_data(state, a_);
+
+ int info;
+ magma_spotrs_gpu(MagmaUpper, n, nrhs, a_data, n, b_data, n, &info);
+
+ // check error value
+ if (info < 0)
+ THError("MAGMA potrs : Argument %d : illegal value", -info);
+
+ THCudaTensor_free(state, b_);
@colesbury
colesbury Mar 21, 2016 Contributor

This should be:

THCudaTensor_freeCopyTo(state, b_, rb_);

(If the destination tensor is provided and not column-major, then the result needs to be copied into it)

@colesbury
Contributor

See in-line comment, but otherwise LGTM. Thanks for adding this!

@bamos
Contributor
bamos commented Mar 22, 2016

Hi @colesbury - thanks! I've fixed freeing b_ and have squashed my commits.

-Brandon.

@soumith soumith merged commit 40d3f52 into torch:master Mar 22, 2016
@soumith
Member
soumith commented Mar 22, 2016

Thanks Brandon!

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