Skip to content

Bound checks in numpy array accessors #584

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

Closed
SylvainCorlay opened this issue Jan 3, 2017 · 3 comments
Closed

Bound checks in numpy array accessors #584

SylvainCorlay opened this issue Jan 3, 2017 · 3 comments

Comments

@SylvainCorlay
Copy link
Member

The current behavior of data access methods in py::array and py::array_t is to check the dimension bounds every time the accessor is called, via the check_dimensions function, event when using it in a loop.

I agree that when exposing a feature to Python, it must be written in such a way that regardless of the values passed to it, it cannot crash the interpreter, but these accessors are not directly exposed to the Python users. Instead, I think that one of the main reasons people would right c++ extensions with pybind is because they want speed, and they might be disappointed with this behavior. #500 already improved the situation, but as we discussed there, I would prefer to completely remove the bound checking.

@aldanor
Copy link
Member

aldanor commented Jan 4, 2017

Which access methods are you most interested in, do you have a minimal example? In a loop, you have unchecked version of byte_offset, plus you can prefetch the data pointer once before the loop starts?

@SylvainCorlay
Copy link
Member Author

SylvainCorlay commented Jan 4, 2017

I think that all the access methods should be like this on the c++ side (except for at which according to the STL idioms (e.g. std::vector) is checked).

I opened this in a separate issue from the original PR hoping that you would reconsider.

The unchecked version are only a few assembly instructions which makes looping worth considering.

@wjakob
Copy link
Member

wjakob commented Mar 22, 2017

Closing this since unchecked access is now provided.

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

No branches or pull requests

3 participants