Re: [Numpy-discussion] Questions about the array interface.
--- Tim Hochberg <tim.hochberg@cox.net> wrote:
I think there is a trade off, but not the one that Chris is worried about. It should be easy to hide complexity of dealing with missing attributes through the various helper functions. The cost will be in speed and will probably be most noticable in C extensions using small arrays where the extra code to check if an attribute is present will be signifigant.
How signifigant this will be, I'm not sure. And frankly I don't care all that much since I generally only use large arrays. However, since one of the big faultlines between Numarray and Numeric involves the former's relatively poor small array performance, I suspect someone might care.
You must check the return value of the PyObject_GetAttr (or PyObject_GetAttrString) calls regardless. Otherwise the extension will die with an ugly segfault the first time one passes an float where an array was expected. If we're talking about small light-weight arrays and a C/C++ function that wants to work with them very efficiently, I'm not convinced that requiring the attributes be present will make things faster. As we're talking about small light weight arrays, it's unlikely the individual arrays will have __array_shape__ or __array_strides__ already stored as tuples. They'll probably store them as a C array as part of their PyObject structure. In the world where some of these attributes are optional: If an attribute like __array_offset__ or __array_shape__ isn't present, the C code will know to use zero or the default C-contiguous layout. So the check failed, but the failure case is probably very fast (since a temporary tuple object doesn't have to be built by the array on the fly). In the world where all of the attributes are required: The array object will have to generate the __array_offset__ int/long or __array_shape___ tuple from it's own internal representation. Then the C/C++ consumer code will bust apart the tuple to get the values. So the check succeeded, but the success code needs to grab the parts of the tuple. The C helper code could look like: struct PyNDArrayInfo { int ndims; int endian; char itemcode; size_t itemsize; Py_LONG_LONG shape[40]; /* assume 40 is the max for now... */ Py_LONG_LONG offset; Py_LONG_LONG strides[40]; /* More Array Info goes here */ }; int PyNDArray_GetInfo(PyObject* obj, PyNDArrayInfo* info) { PyObject* shape; PyObject* offset; PyObject* strides; int ii, len; info->itemsize = too_long_for_this_example(obj); shape = PyObject_GetAttrString(obj, "__array_shape__"); if (!shape) return 0; len = PySequence_Size(shape); if (len < 0) return 0; if (len > 40) return 0; /* This needs work */ info->ndims = len; for (ii = 0; ii<len; ii++) { PyObject* val = PySequence_GetItem(shape, ii); info->shape[ii] = PyLong_AsLongLong(val); Py_DECREF(val); } Py_DECREF(shape); offset = PyObject_GetAttrString(obj, "__array_offset__"); if (offset) { /*** THIS PART MIGHT BE SLOWER WHEN IT SUCCEEDS ***/ info->offset = PyLong_AsLongLong(offset); Py_DECREF(offset); } else { PyErr_Clear(); info->offset = 0; } strides = PyObject_GetAttrString(obj, "__array_strides__"); if (strides) { /*** THIS PART IS ALMOST CERTAINLY SLOWER ***/ for (ii = 0; ii<ndims; ii++) { PyObject* val = PySequence_GetItem(strides, ii); info->strides[ii] = PyLong_AsLongLong(val); Py_DECREF(val); } Py_DECREF(strides); } else { /*** THIS FAILURE PATH IS PROBABLY FASTER ***/ size_t size = info->size; PyErr_Clear(); for (ii = ndims-1; ii>=0; ii--) { info->strides[ii] = size; size *= info->shape[ii]; } } /* More code goes here */ } I have no idea how expensive PyErr_Clear() is. We'd have to profile it to see for certain. If PyErr_Clear() is not expensive, then we could make a strong argument that *not* requiring the attributes will be more efficient. It could also be so close that it doesn't matter - in which case it's back to being a matter of taste... Cheers, -Scott
Scott Gilbert wrote:
--- Tim Hochberg <tim.hochberg@cox.net> wrote:
I think there is a trade off, but not the one that Chris is worried about. It should be easy to hide complexity of dealing with missing attributes through the various helper functions. The cost will be in speed and will probably be most noticable in C extensions using small arrays where the extra code to check if an attribute is present will be signifigant.
How signifigant this will be, I'm not sure. And frankly I don't care all that much since I generally only use large arrays. However, since one of the big faultlines between Numarray and Numeric involves the former's relatively poor small array performance, I suspect someone might care.
You must check the return value of the PyObject_GetAttr (or PyObject_GetAttrString) calls regardless. Otherwise the extension will die with an ugly segfault the first time one passes an float where an array was expected.
If we're talking about small light-weight arrays and a C/C++ function that wants to work with them very efficiently, I'm not convinced that requiring the attributes be present will make things faster.
As we're talking about small light weight arrays, it's unlikely the individual arrays will have __array_shape__ or __array_strides__ already stored as tuples. They'll probably store them as a C array as part of their PyObject structure.
In the world where some of these attributes are optional: If an attribute like __array_offset__ or __array_shape__ isn't present, the C code will know to use zero or the default C-contiguous layout. So the check failed, but the failure case is probably very fast (since a temporary tuple object doesn't have to be built by the array on the fly).
In the world where all of the attributes are required: The array object will have to generate the __array_offset__ int/long or __array_shape___ tuple from it's own internal representation. Then the C/C++ consumer code will bust apart the tuple to get the values. So the check succeeded, but the success code needs to grab the parts of the tuple.
The C helper code could look like:
I'm not convinced it's legit to assume that a failure to get the attribute means that it's not present and call PyErrorClear. Just as a for instance, what if the attribute in question is implemented as a descriptor in which there is some internal error. Then your burying the error and most likely doing the wrong thing. As far as I can tell, the only correct way to do this is to use PyObject_HasAttrString, then PyObject_GetAttrString if that succeeds. The point about not passing around the tuples probably being faster is a good one. Another thought is that requiring tuples instead of general sequences would make the helper faster (since one could use *PyTuple_GET_**ITEM*, which I believe is much faster than PySequence_GetItem). This would possibly shift more pain onto the implementer of the object though. I suspect that the best strategy, orthogonal to requiring all attributes or not, is to use PySequence_Fast to get a fast sequence and work with that. This means that objects that return tuples for strides, etc would run at maximum possible speed, while other sequences would still work. Back to requiring attributes or not. I suspect that the fastest correct way is to require all attributes, but allow them to be None, in which case the default value is used. Then any errors are easily bubbled up and a fast check for None choses whether to use the defaults or not. It's late, so I hope that's not too incoherent. Or too wrong. Oh, one other nitpicky thing, I think PyLong_AsLongLong needs some sort of error checking (it can allegedly raise errors). I suppose that means one is supposed to call PyError_Occurred after every call? That's sort of painful! -tim **
struct PyNDArrayInfo { int ndims; int endian; char itemcode; size_t itemsize; Py_LONG_LONG shape[40]; /* assume 40 is the max for now... */ Py_LONG_LONG offset; Py_LONG_LONG strides[40]; /* More Array Info goes here */ };
int PyNDArray_GetInfo(PyObject* obj, PyNDArrayInfo* info) { PyObject* shape; PyObject* offset; PyObject* strides; int ii, len;
info->itemsize = too_long_for_this_example(obj);
shape = PyObject_GetAttrString(obj, "__array_shape__"); if (!shape) return 0; len = PySequence_Size(shape); if (len < 0) return 0; if (len > 40) return 0; /* This needs work */ info->ndims = len; for (ii = 0; ii<len; ii++) { PyObject* val = PySequence_GetItem(shape, ii); info->shape[ii] = PyLong_AsLongLong(val); Py_DECREF(val); } Py_DECREF(shape);
offset = PyObject_GetAttrString(obj, "__array_offset__"); if (offset) { /*** THIS PART MIGHT BE SLOWER WHEN IT SUCCEEDS ***/ info->offset = PyLong_AsLongLong(offset); Py_DECREF(offset); } else { PyErr_Clear(); info->offset = 0; }
strides = PyObject_GetAttrString(obj, "__array_strides__"); if (strides) { /*** THIS PART IS ALMOST CERTAINLY SLOWER ***/ for (ii = 0; ii<ndims; ii++) { PyObject* val = PySequence_GetItem(strides, ii); info->strides[ii] = PyLong_AsLongLong(val); Py_DECREF(val); } Py_DECREF(strides); } else { /*** THIS FAILURE PATH IS PROBABLY FASTER ***/ size_t size = info->size; PyErr_Clear(); for (ii = ndims-1; ii>=0; ii--) { info->strides[ii] = size; size *= info->shape[ii]; } }
/* More code goes here */ }
I have no idea how expensive PyErr_Clear() is. We'd have to profile it to see for certain. If PyErr_Clear() is not expensive, then we could make a strong argument that *not* requiring the attributes will be more efficient.
It could also be so close that it doesn't matter - in which case it's back to being a matter of taste...
Cheers, -Scott
On Thu, Apr 07, 2005 at 11:55:01PM -0700, Tim Hochberg wrote:
Scott Gilbert wrote:
--- Tim Hochberg <tim.hochberg@cox.net> wrote:
I think there is a trade off, but not the one that Chris is worried about. It should be easy to hide complexity of dealing with missing attributes through the various helper functions. The cost will be in speed and will probably be most noticable in C extensions using small arrays where the extra code to check if an attribute is present will be signifigant.
How signifigant this will be, I'm not sure. And frankly I don't care all that much since I generally only use large arrays. However, since one of the big faultlines between Numarray and Numeric involves the former's relatively poor small array performance, I suspect someone might care.
You must check the return value of the PyObject_GetAttr (or PyObject_GetAttrString) calls regardless. Otherwise the extension will die with an ugly segfault the first time one passes an float where an array was expected.
If we're talking about small light-weight arrays and a C/C++ function that wants to work with them very efficiently, I'm not convinced that requiring the attributes be present will make things faster.
As we're talking about small light weight arrays, it's unlikely the individual arrays will have __array_shape__ or __array_strides__ already stored as tuples. They'll probably store them as a C array as part of their PyObject structure.
In the world where some of these attributes are optional: If an attribute like __array_offset__ or __array_shape__ isn't present, the C code will know to use zero or the default C-contiguous layout. So the check failed, but the failure case is probably very fast (since a temporary tuple object doesn't have to be built by the array on the fly).
In the world where all of the attributes are required: The array object will have to generate the __array_offset__ int/long or __array_shape___ tuple from it's own internal representation. Then the C/C++ consumer code will bust apart the tuple to get the values. So the check succeeded, but the success code needs to grab the parts of the tuple.
The C helper code could look like:
I'm not convinced it's legit to assume that a failure to get the attribute means that it's not present and call PyErrorClear. Just as a for instance, what if the attribute in question is implemented as a descriptor in which there is some internal error. Then your burying the error and most likely doing the wrong thing. As far as I can tell, the only correct way to do this is to use PyObject_HasAttrString, then PyObject_GetAttrString if that succeeds.
No point: PyObject_HasAttrString *calls* PyObject_GetAttrString, then clears the error if there is one. [Side note: hasattr() in Python works the same way, which is why using properties is a pain when you've got code that's using it]
The point about not passing around the tuples probably being faster is a good one. Another thought is that requiring tuples instead of general sequences would make the helper faster (since one could use *PyTuple_GET_**ITEM*, which I believe is much faster than PySequence_GetItem). This would possibly shift more pain onto the implementer of the object though. I suspect that the best strategy, orthogonal to requiring all attributes or not, is to use PySequence_Fast to get a fast sequence and work with that. This means that objects that return tuples for strides, etc would run at maximum possible speed, while other sequences would still work.
How about objects that use a lightweight array as the strides sequence? I'm thinking that if you've got a fast 1-d array object, you'd be tempted to use an instance of that as the shape or strides attribute. You'd be saving on temporary tuple creation (but you'd still be losing some in making Python ints). I haven't benchmarked it, but I'm looking at the code for PySequence_GetItem(): it does a few pointer derefences to get the sq_item() method in the tp_as_sequence struct of an object implementing the sequence protocol, which for the tuple does an array indexing of the tuple's data. You've got about two function calls more compared to using PyTuple_GET_ITEM. It really depends on how big the arrays you expect to get passed to you. If they're big, this is all amortized: you'll hardly see it. It also depends on how your routines get used. If the routine is buried below a few layers of API, you'd likely be better off doing a typecast higher up to your own representation, or something. If it's at the border, so the user will call it directly *often*, you're going to be screwed for speed anyways (giving the user the option of casting arrays to something else would probably help a lot here also).
Back to requiring attributes or not. I suspect that the fastest correct way is to require all attributes, but allow them to be None, in which case the default value is used. Then any errors are easily bubbled up and a fast check for None choses whether to use the defaults or not.
It's late, so I hope that's not too incoherent. Or too wrong.
Oh, one other nitpicky thing, I think PyLong_AsLongLong needs some sort of error checking (it can allegedly raise errors). I suppose that means one is supposed to call PyError_Occurred after every call? That's sort of painful!
Yes! Check all C API functions that may return errors! That includes PySequence_GetItem() and PyLong_AsLongLong.
struct PyNDArrayInfo { int ndims; int endian; char itemcode; size_t itemsize; Py_LONG_LONG shape[40]; /* assume 40 is the max for now... */ Py_LONG_LONG offset; Py_LONG_LONG strides[40]; /* More Array Info goes here */ };
int PyNDArray_GetInfo(PyObject* obj, PyNDArrayInfo* info) { PyObject* shape; PyObject* offset; PyObject* strides; int ii, len;
info->itemsize = too_long_for_this_example(obj);
shape = PyObject_GetAttrString(obj, "__array_shape__"); if (!shape) return 0; len = PySequence_Size(shape); if (len < 0) return 0; if (len > 40) return 0; /* This needs work */ info->ndims = len; for (ii = 0; ii<len; ii++) { PyObject* val = PySequence_GetItem(shape, ii);
info->shape[ii] = PyLong_AsLongLong(val);
and here
Py_DECREF(val);
(if you don't check PySequence_GetItem -- not a good idea anyways --
Like here this should be Py_XDECREF) [snip more code that needs checks :-)]
I have no idea how expensive PyErr_Clear() is. We'd have to profile it to see for certain. If PyErr_Clear() is not expensive, then we could make a strong argument that *not* requiring the attributes will be more efficient.
Not much; it's about three Py_XDECREF's. -- |>|\/|< /--------------------------------------------------------------------------\ |David M. Cooke http://arbutus.physics.mcmaster.ca/dmc/ |cookedm@physics.mcmaster.ca
--- "David M. Cooke" <cookedm@physics.mcmaster.ca> wrote:
Oh, one other nitpicky thing, I think PyLong_AsLongLong needs some sort
of error checking (it can allegedly raise errors). I suppose that means one is supposed to call PyError_Occurred after every call? That's sort of painful!
Yes! Check all C API functions that may return errors! That includes PySequence_GetItem() and PyLong_AsLongLong.
Sorry, I should have been clear that I was writing example code. I only put the error checking in where I thought it was demonstrating the point. I'd be surprized if it even compiled... Note that the additional error checking is required in the "success" path where the attributes are present. In other words, mandating the attributes be there when they aren't strictly required could make things slower... Cheers, -Scott
--- Tim Hochberg <tim.hochberg@cox.net> wrote:
The point about not passing around the tuples probably being faster is a good one. Another thought is that requiring tuples instead of general sequences would make the helper faster (since one could use *PyTuple_GET_**ITEM*, which I believe is much faster than PySequence_GetItem). This would possibly shift more pain onto the implementer of the object though. I suspect that the best strategy, orthogonal to requiring all attributes or not, is to use PySequence_Fast to get a fast sequence and work with that. This means that objects that return tuples for strides, etc would run at maximum possible speed, while other sequences would still work.
I hadn't seen this "fast" sequence stuff before. Thanks for the pointer.
Back to requiring attributes or not. I suspect that the fastest correct way is to require all attributes, but allow them to be None, in which case the default value is used. Then any errors are easily bubbled up and a fast check for None choses whether to use the defaults or not.
How about saying that, for all the optional attributes, if they return None that's to be treated the same way as if they weren't present at all? In other words, they're still optional, but people in the know would know that returning None was probably faster...
participants (3)
-
David M. Cooke -
Scott Gilbert -
Tim Hochberg