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

add a bitequal() function #9

Open
wants to merge 195 commits into
base: master
Choose a base branch
from
Open

Conversation

andre-merzky
Copy link

I added a bitequal() function to calculate the same as (a & b).count().

Thanks for this great library.
Björn


This PR was moved from ilanschnell/pull/8.

flowerhack and others added 6 commits October 7, 2014 13:40
Pythons list implementation has a special case to ensure that
x.extend(x) is the same as x.extend(list(x)). i.e. it doesn't notice
changes to the size of the list during the extend.

Previously this case would not have been handled correctly by
bitarray. This changes that by checking what the size of the other
array is before we make any changes. This ensures both that this works
correctly and also (importantly) that we don't try to write past the
end of the array.
@diamondman
Copy link
Owner

I love the left and right shift. But I think we should make theseoperator overloads so we can >> or <<

@diamondman
Copy link
Owner

@udoprog @eltoder I assume equals can be sped up with SIMD. Can left/right shift or equal benefit SIMD as well? I could see issues of shifting more than the vector size.

@eltoder
Copy link

eltoder commented Oct 23, 2016

@diamondman Yes, bitdiff() may now be slower than (a ^ b).count(), since the latter uses SIMD. Ideally we should have a SIMD version of count(), bitdiff() should use SIMD and that code as well, and this new bitequal() should reuse as much of that code as possible, instead of being a copy-paste.

Agreed that lshift() and rshift() should be hooked to python's >> and << instead. Also, these versions claim that they only work on big endian machines, and are not tested, which seems bad.

As for using SIMD for shifts: in short, it only works for constant shifts or multiples of 8. If we use attribute vector_size, we can see what the compiler can do for us, but I don't expect much.

@diamondman
Copy link
Owner

I agree the big endian only is a problem. Maybe there is something woth doing with shifting with SIMD multiples of 8 and then doing the rest of shiftcount%8 shift as a second operation. It would cause double writes, but maybe it would be faster than an arbitrary shift operation. I will try to look into this in a bit.

@diamondman
Copy link
Owner

The code in this PR does move around the bytes first, so that is a spot for SIMD! 0fd042e#diff-1997c64b6c87f3a5197c717a65138709R2412

}

#define aa ((bitarrayobject *) a)
#define bb ((bitarrayobject *) b)
Copy link

@udoprog udoprog Oct 23, 2016

Choose a reason for hiding this comment

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

Should prefer using variables here.

@@ -2925,6 +3079,48 @@ static PyTypeObject Bitarraytype = {
/*************************** Module functions **********************/

static PyObject *
bitequal(PyObject *self, PyObject *args)
Copy link
Owner

Choose a reason for hiding this comment

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

Why is this a module method instead of being on the instance?

Copy link

Choose a reason for hiding this comment

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

I'm guessing because bitdiff() is not a method. Arguably they both should be methods, and the names could be more clear, like count_equal() and count_different().

@rrrealman
Copy link

Hi guys, is there still need in simple shifts? As I found here, it was not merged. So, I need this implementation and have time to commit it.

@andre-merzky
Copy link
Author

I think it would be a shame to let this PR deteriorate further...

@rrrealman
Copy link

You are right. I'll create a new one so I have it already implemented.

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.