Add all the missing error messages and stop filtering in pygpu.#449
Add all the missing error messages and stop filtering in pygpu.#449abergeron merged 10 commits intoTheano:masterfrom
Conversation
| if (k == NULL) | ||
| return error_set(ctx->err, GA_MISC_ERROR, | ||
| "Could not instantiate GpuElemwise copy kernel"); | ||
| return ctx->err->code; |
There was a problem hiding this comment.
GpuElemwise_new do not always set the error message.
|
|
||
| if (!GpuArray_ISWRITEABLE(a)) | ||
| return GA_VALUE_ERROR; | ||
| return error_set(ctx->err, GA_INVALID_ERROR, "Destination array not writable"); |
There was a problem hiding this comment.
Why did you change the error type? I think the old one is better.
| } | ||
|
|
||
| if (newsize != oldsize) return GA_INVALID_ERROR; | ||
| if (newsize != oldsize) return error_set(ctx->err, GA_INVALID_ERROR, "New shope differs in total size"); |
| free(buf); | ||
| fprintf(fd, "<unsupported data type %d>\n", a->typecode); | ||
| return GA_UNSUPPORTED_ERROR; | ||
| return error_set(ctx->err, GA_UNSUPPORTED_ERROR, "Unsupported data type for dump"); |
There was a problem hiding this comment.
If you can remove the fprintf above and put the typecode inside the error, it would be better.
There was a problem hiding this comment.
Since this is a debugging help, I want it to be chatty when the data type is not supported.
I've added the typecode to the error message.
| if (A->typecode != GA_HALF && A->typecode != GA_FLOAT && | ||
| A->typecode != GA_DOUBLE) | ||
| return GA_INVALID_ERROR; | ||
| return error_set(ctx->err, GA_INVALID_ERROR, "Unsupported type"); |
There was a problem hiding this comment.
I don't like that you use the work "type" for the dtype/typecode. Why you don't use dtype?
There was a problem hiding this comment.
I changed them to dtype.
| } | ||
| } else { | ||
| err = GA_VALUE_ERROR; | ||
| err = error_set(ctx->err, GA_VALUE_ERROR, "Invalid internal result for B"); |
| return error_set(ctx->err, GA_VALUE_ERROR, "Type mismatch"); | ||
| if (!GpuArray_ISALIGNED(src) || !GpuArray_CHKFLAGS(dest, GA_BEHAVED)) | ||
| return GA_UNALIGNED_ERROR; | ||
| return error_set(ctx->err, GA_UNALIGNED_ERROR, "Misbehaved arrays"); |
There was a problem hiding this comment.
unaligned or misbehaved.
I don't remember what is misbehaved myself, so we can't expect user to also remember. I think you should split that into 2 different error and give a better error for the misbehaved version.
There was a problem hiding this comment.
BEHAVED == ALIGNED && WRITABLE
| ktypes = calloc(p, sizeof(int)); | ||
| if (ktypes == NULL) | ||
| if (ktypes == NULL) { | ||
| res = error_fmt(ctx->err, "calloc"); |
| for (j = 0; j < a->nd; j++) { | ||
| if (v->dimensions[j] != a->dimensions[j]) | ||
| return GA_VALUE_ERROR; | ||
| return error_fmt(ctx->err, GA_VALUE_ERROR, "Mismatched dimension %u", j); |
There was a problem hiding this comment.
Can you add the values of that dimensions? It really help debug.
| return error_sys(ctx->err, "malloc"); | ||
| if (tmp == NULL) { | ||
| error_sys(src_ctx->err, "malloc"); | ||
| return error_sys(dst_ctx->err, "malloc"); |
There was a problem hiding this comment.
If they are the same context, I suppose it is fine to set the error twice. The slow down is probably not important.
| array_index(res, a, starts, stops, steps) | ||
| except ValueError, e: | ||
| raise IndexError, "index out of bounds" | ||
| array_index(res, a, starts, stops, steps) |
There was a problem hiding this comment.
I think this is the reason for the newly-failing test in Theano/Theano#6026 (comment)
I'm not sure what is happening, but presumably what used to be an IndexError was catched somewhere, and now it is a Value Error.
3 participants
I hope I did not miss a spot.