Skip to content
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

Airspeed velocity benchmarks #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

hameerabbasi
Copy link

No description provided.

@hameerabbasi hameerabbasi requested a review from skrah April 15, 2019 15:58
@skrah
Copy link
Member

skrah commented Apr 16, 2019

Thanks, this takes a bit of time to comprehend. I have a couple of remarks:

  1. I find asv unsuitable for interactive use, so I prefer to leave the original files. Perhaps put everything in an asv directory and the original .py files into a scripts directory.

  2. I couldn't find the reason why the tuple access result in asv differs so much from the loop result:

Accessing an element in an array of tuples
------------------------------------------

   xnd:   0.2511618137359619
   numpy: 0.7269613742828369
  1. Is there some sort of make clean for asv?

@hameerabbasi
Copy link
Author

Thanks, this takes a bit of time to comprehend. I have a couple of remarks:

It is pretty thick code, I agree.

  1. I find asv unsuitable for interactive use, so I prefer to leave the original files. Perhaps put everything in an asv directory and the original .py files into a scripts directory.

Done.

  1. I couldn't find the reason why the tuple access result in asv differs so much from the loop result:

I'm pretty sure it's because the Python object has to be constructed each time you do something like a[0], whereas in the tuple access, everything can be done in the C level and then accessed from Python once. It may also be the overhead of the loop itself.

  1. Is there some sort of make clean for asv?

There is asv rm bit it just cleans the results for previous commits' results, not the entire environment tree.

I would just use git clean -xfd ..

@skrah
Copy link
Member

skrah commented Apr 16, 2019

I'd like to go to the bottom of the issue of different results for tuple element access. I don't think it's a loop issue, because all access benchmarks have the same loop.

One reason might be that xnd generates views also for elements and numpy does not (by default). So that would be an advantage for xnd that should show up in the benchmarks.

Are you sure that single elements and not subtrees are accessed (I don't have time to really dig into the code)?

@hameerabbasi
Copy link
Author

Oh. If dim[1] < dim[0], it's a subtree access. If dim[1] == dim[0], an element is accessed (In NumPy), and an element-view (In XND).

@hameerabbasi
Copy link
Author

Actually, in the for loop version, the last iteration of the loop in NumPy accesses a scalar (for dim[0] == dim[1]).

dim[0] == array.ndim, dim[1] is the number of integer indices.

Copy link
Author

@hameerabbasi hameerabbasi left a comment

Choose a reason for hiding this comment

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

To expand on the previous issue.

self.array = globals()[module].array(lst)

def time_access_tuple(self, size, dim, module):
self.array[(0,) * dim[1]]
Copy link
Author

@hameerabbasi hameerabbasi Apr 16, 2019

Choose a reason for hiding this comment

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

Here's the access for tuple access. Equivalent to a[0, 0, ...] where the ellipsis is a continuation and not a Python ellipsis.

Copy link
Member

@skrah skrah Apr 16, 2019

Choose a reason for hiding this comment

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

NumPy does not appear to be able to use multi-indexing on tuples:

>>> import numpy as np
>>> lst = [('Rex', 9, 81.0)] * 100
>>> dt = np.dtype([('name', 'U10'), ('age', 'i4'), ('weight', 'f4')])
>>> x = np.array(lst, dtype=dt)
>>> y = x[0][0]
>>> type(y)
<class 'numpy.str_'>
>>> 
>>> y = x[0, 0]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: too many indices for array

Copy link
Author

Choose a reason for hiding this comment

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

I don't test tuples in the element access at all, I could add that.

Copy link
Member

@skrah skrah Apr 16, 2019

Choose a reason for hiding this comment

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

The fast benchmark measures y = x[0][0], i.e. it accesses the 'Rex' tuple element, not the array element.

Copy link
Member

@skrah skrah Apr 16, 2019

Choose a reason for hiding this comment

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

xnd is able to access through tuples:

>>> lst = [('Rex', 9, 81.0)] * 100
>>> x = xnd(lst)
>>> x[0, 0]
xnd('Rex', type='string')

So that should be even faster for xnd.

self.array[(0,) * dim[1]]

def time_access_chained(self, size, dim, module):
a = self.array
Copy link
Author

Choose a reason for hiding this comment

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

Here, I cache a, then in a loop, access it. This is equivalent to a[0][0]...

@skrah
Copy link
Member

skrah commented Apr 16, 2019

In general I think people are likely to write benchmarks for small arrays. xnd is currently at a disadvantage because the python convenience classes in __init__.py are not optimized for small arrays.

Here is the construction benchmark when using the C Xnd constructor directly on a small one element list:

from xnd import Xnd
...
for i in range(repeat):
    x = Xnd(ndt("1 * int64"), lst)
...
Dtype provided
--------------

   xnd:   0.07504105567932129
   numpy: 0.08943438529968262

@skrah
Copy link
Member

skrah commented Apr 16, 2019

So I'll rewrite the class in C now before publishing the benchmarks.

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.

None yet

2 participants