4
\$\begingroup\$

As part of a game engine I am creating, I am in the process of writing a bunch of math classes (vector2/3/4, matrix2/3/4, and quaternions to start with).

Vec2 class:

import math
import random

class Vec2(object):
    def __init__(self, x, y):
        self._x = float(x)
        self._y = float(y)

    @property
    def x(self):
        return self._x

    @x.setter
    def x(self, new_x):
        self._x = float(new_x)

    @property
    def y(self):
        return self._y

    @y.setter
    def y(self, new_y):
        self._y = float(new_y)

    def __add__(self, other):
        types = (int, float)
        if isinstance(self, types):
            return Vec2(self + other.x, self + other.y)
        elif isinstance(other, types):
            return Vec2(self.x + other, self.y + other)
        else:
            return Vec2(self.x + other.x, self.y + other.y)

    def __div__(self, other):
        types = (int, float)
        if isinstance(self, types):
            self = Vec2(self, self)
        elif isinstance(other, types):
            other = Vec2(other, other)
        x = self.x / other.x
        y = self.y / other.y
        return Vec2(x, y)

    def __mul__(self, other):
        types = (int, float)
        if isinstance(self, types):
            return Vec2(self * other.x, self * other.y)
        elif isinstance(other, types):
            return Vec2(self.x * other, self.y * other)
        else:
            return Vec2(self.x * other.x, self.y * other.y)

    def __neg__(self):
        return Vec2(-self.x, -self.y)

    def __radd__(self, other):
        return Vec2(self.x + other, self.y + other)

    def __rdiv__(self, other):
        return Vec2(other/self.x, other/self.y)

    def __rmul__(self, other):
        return Vec2(other * self.x, other * self.y)

    def __rsub__(self, other):
        return Vec2(other - self.x, other - self.y)

    def __repr__(self):
        return self.__str__()

    def __str__(self):
        return "Vec2: ({0}, {1})".format(self.x, self.y)

    def __sub__(self, other):
        types = (int, float)
        if isinstance(self, types):
            return Vec2(self - other.x, self - other.y)
        elif isinstance(other, types):
            return Vec2(self.x - other, self.y - other)
        else:
            return Vec2(self.x - other.x, self.y - other.y)

    def ceil(self):
        return Vec2(math.ceil(self.x), math.ceil(self.y))

    def floor(self):
        return Vec2(math.floor(self.x), math.floor(self.y))

    def get_data(self):
        return (self.x, self.y)    

    def inverse(self):
        return Vec2(1.0/self.x, 1.0/self.y)

    def length(self):
        return math.sqrt(self.square_length())

    def normalize(self):
        length = self.length()
        if length == 0.0:
            return Vec2(0, 0)
        return Vec2(self.x/length, self.y/length)

    def round(self):
        return Vec2(round(self.x), round(self.y))

    def square_length(self):
        return (self.x * self.x) + (self.y * self.y)

    """
    def transform(self, matrix):#mat2, mat2d, mat3, mat4
        pass

    @classmethod
    def cross(cls, a, b):
        z = (a.x * b.y) - (a.y * b.x)
        return Vec3(0, 0, z)
    """

    @classmethod
    def distance(cls, a, b):
        c = b - a
        return c.length()

    @classmethod
    def dot(self, a, b):
        return (a.x * b.x) + (a.y * b.y)

    @classmethod
    def equals(cls, a, b, tolerance=0.0):
        diff = a - b
        dx = math.fabs(diff.x)
        dy = math.fabs(diff.y)
        if dx <= tolerance * max(1, math.fabs(a.x), math.fabs(b.x)) and \
           dy <= tolerance * max(1, math.fabs(a.y), math.fabs(b.y)):
            return True
        return False

    @classmethod
    def max(cls, a, b):
        x = max(a.x, b.x)
        y = max(a.y, b.y)
        return Vec2(x, y)

    @classmethod
    def min(cls, a, b):
        x = min(a.x, b.x)
        y = min(a.y, b.y)
        return Vec2(x, y)

    @classmethod
    def mix(cls, a, b, t):
        return a * t + b * (1-t)

    @classmethod
    def random(cls):
        x = random.random()
        y = random.random()
        return Vec2(x, y)

    @classmethod
    def square_distance(cls, a, b):
        c = b - a
        return c.square_length()

And here is how it can currently be used in my engine:

from pyorama.math3d.vec2 import Vec2

a = Vec2(1, 2.5)
b = Vec2(5, -1)
print a+1, 1+a, a+b, b+a
print a-1, 1-a, a-b, b-a
print a*2, 2*a, a*b, b*a
print a/3, 3/a, a/b, b/a
print -a, -b

a.x = 3.0
a.y = 4
print type(a.x), type(a.y), a

print a.ceil()
print a.floor()
print a.get_data()
print a.inverse()
print a.length()
print a.normalize()
print a.round()
print a.square_length()
#print a.transform(None)#not implemented
#print Vec2.cross(a, b)#not implemented
print Vec2.distance(a, b)
print Vec2.dot(a, b)
print Vec2.equals(a, b, tolerance=200)
print Vec2.max(a, b)
print Vec2.min(a, b)
print Vec2.mix(a, b, 0.5)
print Vec2.random()
print Vec2.square_distance(a, b)

My major concern is whether or not it would be better to organize the code as it is currently, in a module, in a purely OOP manner (without @classmethod) or in a @classmethod only manner. Here are the advantages as I see them:

  • My current organization is inconsistent as it mixes instance (when there is one Vec2 argument) and class methods (when there are two Vec2 arguments) together. This might be confusing for new users, but it could make writing math out in code much easier.
  • A vec2 module instead could lead to better performance (since I am not creating a bunch of Vec2 objects and would be returning tuples/lists instead). However, I would not have any niceties such as operator overloading.
  • Avoiding @classmethod would make the code more consistent and could allow chaining, but it may be difficult to read a.dot(b) versus Vec2.dot(a, b).
  • Using only @classmethod would make the code more consistent in usage (everything is Vec2.something() but chaining operations would be impossible. Operator overloading could also be annoying to deal with.

Another minor issue I have is with the names of my functions. Is it better to abbreviate the names and replace Vec2.distance(a, b) with Vec2.dist(a, b) in order to be consistent with the abbreviated name of the class or should I leave them spelled out instead (and perhaps rename Vec2 to Vector2)? I was also not sure if is it acceptable to use built-in function names in my custom class (such as len or round) as while I am not polluting the namespace and I cannot think of adequate alternative names, it can lead to improper syntax highlighting.

I know I am asking a lot of probably trivial stylistic questions, but I would greatly appreciate any advice!


Following @ChrisR's helpful idea of creating base vector and matrix classes using numpy that can be extended, I have created a Vec class and a Vec2 class. Here are both of those classes as they currently stand:

vec.py:

import math
import numpy as np
import random

class Vec(object):
    def __init__(self, data):
        self.data = data

    def __add__(self, other):
        if isinstance(other, Vec):
            return Vec(self.data + other.data)
        return Vec(self.data + other)

    def __radd__(self, other):
        return Vec(other + self.data)

    def __sub__(self, other):
        if isinstance(other, Vec):
            return Vec(self.data - other.data)
        return Vec(self.data - other)

    def __rsub__(self, other):
        return Vec(other - self.data)

    def __mul__(self, other):
        if isinstance(other, Vec):
            return Vec(self.data * other.data)
        return Vec(self.data * other)

    def __rmul__(self, other):
        return Vec(other * self.data)

    def __div__(self, other):
        if isinstance(other, Vec):
            return Vec(self.data / other.data)
        return Vec(self.data / other)

    def __rdiv__(self, other):
        return Vec(other / self.data)

    def __neg__(self):
        return Vec(-self.data)

    def __pos__(self):
        return Vec(+self.data)

    def __eq__(self, other):
        return np.array_equal(self.data, other.data)

    def __ne__(self, other):
        return not self.__eq__(other)

    def __lt__(self, other):
        return self.square_length() < other.square_length()

    def __le__(self, other):
        return self.square_length() <= other.square_length()

    def __gt__(self, other):
        return self.square_length() > other.square_length()

    def __ge__(self, other):
        return self.square_length() >= other.square_length()

    def __repr__(self):
        return self.__str__()

    def __str__(self):
        return np.array_str(self.data)

    def ceil(self):
        return Vec(np.ceil(self.data))

    def floor(self):
        return Vec(np.floor(self.data))

    def get_data(self):
        return self.data

    def inverse(self):
        return Vec(1.0/self.data)

    def length(self):
        return float(np.linalg.norm(self.data))

    def normalize(self):
        length = self.length()
        if length == 0.0:
            return Vec(np.zeros(self.data.shape()))
        return Vec(self.data/length)

    def round(self, decimal=0):
        return Vec(np.round(self.data, decimal))

    def square_length(self):
        return float(np.sum(np.square(self.data)))

    @classmethod
    def distance(cls, a, b):
        c = b - a
        return c.length()

    @classmethod
    def dot(self, a, b):
        return Vec(np.dot(a.data, b.data))

    @classmethod
    def equals(cls, a, b, tolerance=0.0):
        diffs = np.fabs((a - b).data)
        pairs = zip(list(np.fabs(a.data)), list(np.fabs(b.data)))
        tolerance_calcs = [tolerance * max(1, a_val, b_val) for (a_val, b_val) in pairs]
        tests = [d <= t for (d, t) in zip(diffs, tolerance_calcs)]
        return all(tests)

    @classmethod
    def max_components(cls, a, b):
        return Vec(np.maximum(a.data, b.data))

    @classmethod
    def min_components(cls, a, b):
        return Vec(np.minimum(a.data, b.data))

    @classmethod
    def mix(cls, a, b, t):
        return a*(1-t) + b*t

    @classmethod
    def random(cls, n):
        x = random.random()
        y = random.random()
        return Vec(np.random.rand((n)))

    @classmethod
    def square_distance(cls, a, b):
        c = b - a
        return c.square_length()

vec2.py:

from vec import Vec

import math
import numpy as np
import random

class Vec2(Vec):
    def __init__(self, x, y):
        self._x = float(x)
        self._y = float(y)
        super(Vec2, self).__init__(np.array([x, y], dtype=np.float32))

    @property
    def x(self):
        return self._x

    @x.setter
    def x(self, new_x):
        self._x = float(new_x)
        self.data[0] = self._x

    @property
    def y(self):
        return self._y

    @y.setter
    def y(self, new_y):
        self._y = float(new_y)
        self.data[1] = self._y

This is definitely a lot more convenient, although the numpy arithmetic leads to some interesting floating point rounding errors. I renamed min/max to min_components/max_components to clear up any confusion and added > and < comparisons that compare based on vector magnitude.

However, this does introduce a new issue. One is the random function in Vec2 needs to be called like Vec2.random(2) which is ugly. In that case, should I even implement a random function in the base Vec class, or is there a way to keep both Vec.random(n) as well as Vec2.random()?

Additionally, I was trying to move functions like dot to the module level, but then wouldn't users have to call both of these imports if they were dotting two Vec2 vectors:

from pyorama.math3d.vec import dot
from pyorama.math3d.vec2 import Vec2

I would rather that users not even touch the base Vec class at all! Is there a cleaner way to do imports if I switch the dot function code to a module level, either by doing some import trickery in the __init__.py file in the math3d folder where all this code is or in vec.py or vec2.py?


For anyone interested in what the "final" code looks like, I now have completed writing code for the Vec2/3/4, Mat2/3/4, and Quat classes. This can all be found in this github repository under the math3d folder. Thanks for all of your feedback and help!

\$\endgroup\$
  • \$\begingroup\$ Is there any specific reason to rewrite this kind of library instead of using numpy for example? It would surely simplify your implementation of quaternions especially if you'll be computing time derivatives of your quaternions. \$\endgroup\$ – ChrisR Dec 30 '16 at 1:48
  • \$\begingroup\$ @ChrisR I have used numpy before and enjoyed it but it is not as convenient to use in the context of a game. For instance, for a 4x4 matrix, numpy doesn't provide any rotation, lookAt, or perspective methods. Plus it didn't make much sense to make a general numpy array behind the scenes and bring in that overhead when the arrays are so small and numpy's vectorization benefits would not be seen (for a vec2, it is just two items). I am basically trying to recreate something like the glm library for c++ or glmatrix library for javascript. \$\endgroup\$ – CodeSurgeon Dec 30 '16 at 2:35
  • 2
    \$\begingroup\$ I see. But doesn't that mean you'll have a ton of code duplication between your Vec2-4 classes if you don't want to use arrays? The way I would tackle the problem is to use both your custom implementation and numpy: have a super class which generates the correctly sized vector/matrix, and store all the data in a numpy array. Then for anything which isn't supported by numpy, do your own implementation. It also limits the scope of what you need to write tests for. \$\endgroup\$ – ChrisR Dec 30 '16 at 3:25
  • 2
    \$\begingroup\$ @ChrisR The super class approach is definitely the right one in this case! A quick parsing of all of the function names in the glmatrix source shows that there is a lot of overlap. I will go ahead and implement a generic vec class tonight along with an extended vec2 class and update the question. I will also have to figure out how to do x, y, z, and w properties for each of the special cases of the vec class. Do you have any advice about the function names and the use/abuse of @classmethod? \$\endgroup\$ – CodeSurgeon Dec 30 '16 at 4:57
  • 2
    \$\begingroup\$ Also, I'm slightly confused at your min and max functions: they return the min/max of each component of the vector. Is that your plan? Just from the name of these functions, I would expect them to return the vector with the smallest or greatest absolute value. If the latter is the goal, I would recommend you to overload the operator functions for less than and greater than. That will allow you to use the built-in min and max functions with any number of parameters which are all of the same type. \$\endgroup\$ – ChrisR Dec 30 '16 at 5:18
3
\$\begingroup\$

In your first version, I don't understand your various

if isinstance(self, types):

checks. Since you are building the Vec2 class, you know that self is of type Vec2 or one of its subclasses, if any. So these checks will always evaluate to False: you’d better remove them.

But as you figured out with the second version, you can remove a lot of repetitions with a base class or some kind of factory.

However, what seems a big mistake in the second version is the fact that you are downcasting any VecX to a Vec after the first arithmetic operation. Meaning that

v1 = Vec2(4, 8)
v2 = Vec2(1, 1)
v3 = v1 + v2
print(v3.x, v3.y)

Will raise AttributeError for the v3.x and v3.y accesses. Because v3 is a Vec and not a Vec2.

To avoid that, I don't have any better idea than to hack the __init__ method a bit:

def Vector(n):
    class Vec(object):
        def __init__(self, *args):
            try:
                data, = args
            except ValueError:
                data = np.array(args, dtype=np.float32)
            assert len(data) == n
            self.data = data
        # other common methods
    return Vec

class Vec2(Vector(2)):
    # special methods for Vec2

This allows you to call Vec2(data) where data is a numpy array as well as Vec2(2, 0.8).

This means that, in your various operators, you will need to return the right type, something along the lines of return self.__class__(self.data + other.data) should do, for the addition.


The last thing that I wanted to mention is your usage of the properties. Why manage a separate variable when you have direct access to the underlying data structure? I would simplify Vec2 to:

class Vec2(Vector(2)):
    @property
    def x(self):
        return self.data[0]

    @x.setter
    def x(self, value):
        self.data[0] = value  # No need to manually convert to float, numpy will do it for us

    @property
    def y(self):
        return self.data[1]

    @y.setter
    def y(self, value):
        self.data[1] = value
\$\endgroup\$
  • \$\begingroup\$ Definitely agree with most of what you are saying. The isinstance checks in the old version made no sense and there is no point of keeping things like self._x floating around in the code when using property decorators. You are right about the downcasting issue, but I will have to think about your solution for a bit. Would it just be possible to modify Vec directly rather than introducing Vector? \$\endgroup\$ – CodeSurgeon Dec 30 '16 at 16:58
  • \$\begingroup\$ @CodeSurgeon The only use of the Vector "factory" is the ability to assert len(data) == n. Some other optimizations may depend on n, I don't know. If you don't find any and/or don't need the assert, you can safely remove the function in favor of a single base class. \$\endgroup\$ – Mathias Ettinger Dec 30 '16 at 17:05
  • \$\begingroup\$ I think I can eliminate the Vector factory. Then in the Vec2 class, I can override each inherited function by either calling the corresponding parent Vec function with super and casting the results by returning a Vec2 object at the end using return Vec2(*result.data) or define some custom behavior (which may be useful for say handling transforming matrices of various sizes). \$\endgroup\$ – CodeSurgeon Dec 30 '16 at 19:33
  • 2
    \$\begingroup\$ @CodeSurgeon you don't need to cast (and thus, to override) if you use something like return self.__class__(self.data + other.data) instead of return Vec(self.data + other.data) \$\endgroup\$ – Mathias Ettinger Dec 30 '16 at 19:43
  • 2
    \$\begingroup\$ @CodeSurgeon You'll note that I don't define an __init__ in Vec2. Vec already handle being passed a single parameter being a numpy array or n parameters that will be converted to an array. \$\endgroup\$ – Mathias Ettinger Dec 30 '16 at 19:58

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.