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 & speed up PVector operations, clean up namespace #44

Merged
merged 4 commits into from Apr 21, 2014

Conversation

kazimuth
Copy link
Contributor

Another pull request!
This does a few nice semantic things for PVector:

  • Adds an __rmul__ method for PVector, so that you can say 5 * PVector(2, 3, 4) without receiving a TypeError.
  • Adds a __div__ method for PVector so that they can be divided by scalars (as a shorthand for (1/a) * v).

It also does a few performance things:

  • Uses native java methods for PVector operations instead of python methods.
  • Specializes incremental operators (__iadd__ for += and so on) for PVector so that they update the vector in-place instead of allocating new copies.

Normally, I wouldn't worry about microoptimizing like this, but vectors are the sort of things that could be modified in hot loops, and it's a simple change. According to a completely unscientific benchmark I rigged up (lots of vector operations with timeit()), using Java code for * makes it twice as fast, and destructively updating a variable with *= is four times as fast.

Finally, this cleans up the functions after they're patched onto PVector, since they were still hanging out in the global namespace.

@jdf
Copy link
Owner

jdf commented Apr 20, 2014

Fantastic; thank you. Maybe add a few lines to the PVector unit tests?

@kazimuth
Copy link
Contributor Author

Sure thing.

@kazimuth
Copy link
Contributor Author

There's tests!
Also, usability question: would it make sense to call float() on the scalar arguments to __mul__(a, b) and so on? I found out that it gives a TypeError: unsupported operand type(s) for /: 'processing.core.PVector' and 'int' right now if you divide by, say, 5, rather than 5.0. Java will transparently convert integers to floats (even if they're explicitly declared as int), but python apparently doesn't, and it would be nice to make it so. On the other hand, I could see it giving wacky error messages (or working when it shouldn't!) if someone passed in objects or strings or something.

@jdf
Copy link
Owner

jdf commented Apr 20, 2014

Yes, please do use the float() casting function on all operands. I'd definitely consider it a bug if dividing by the literal 5 caused a crash. Then add at least one test to explicitly check that dividing a PVector by an int literal works as expected. I'd much rather have it weirdly succeed on a "5" than fail on a 5.

@kazimuth
Copy link
Contributor Author

Will do- I'll check it in in the morning

@kazimuth
Copy link
Contributor Author

Done - tests pass.

jdf pushed a commit that referenced this pull request Apr 21, 2014
Add & speed up PVector operations, clean up namespace
@jdf jdf merged commit 704f2d3 into jdf:master Apr 21, 2014
@jdf
Copy link
Owner

jdf commented Apr 21, 2014

Many thanks.

@kazimuth kazimuth deleted the more-pvector-ops branch April 21, 2014 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants