[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [pygame] Re: removing 'experimental' notice from pygame.math



Some good points. Thanks, I appreciate the discussion.


On Tue, Feb 27, 2018 at 3:51 PM, Daniel Pope <mauve@xxxxxxxxxxxxxx> wrote:
> Single scalar constructor should Vector2(1) == Vector2(1, 1)

This is the kind of situation where I'd follow PEP20: "In the face of ambiguity, refuse the temptation to guess." There's an obvious pitfall if you expected to pass a tuple and you instead pass a scalar or the wrong length tuple - it silently succeeds but constructs the wrong Vector. Perhaps raise a TypeError?


At the moment Vector2/3 uses keyword arguments.
Vector2() -> Vector(0, 0)
​Vector2(x=0, y=0) -> Vector(0, 0)
Vector2(1) -> Vector(1, 0)

I'm beginning to think keyword arguments are wrong for this.
Would maybe prefer to raise a TypeError as you suggest,
  or follow GLSL behaviour and allow single scalar, error for the others.

These should raise an error for sure?
  Vector3(1, 1) # oops, I meant to type three 1s.
  Vector3(some2d_I_thought_was_a_3d) #oops, the incoming value is of a different size.

However, I feel this single scalar is hard to make
a mistake with accidentally passing in the wrong thing?
  Vector(1)

Keyword arguments are also pretty useless for readability with Vector2/3.
Because you know it's a 3 element container.
  Vector3(x=0, y=0, z=0) VS
  Vector3(0,0,0)

It's fairly common to init vec2/vec3 in GLSL with a single number.
Eyeballing a few scripts, maybe 10% of usage is single scalar constructor.
And to be able to have fairly good compatibility with code from shadertoy might be nice.





> Lower dimensional to higher dimension contructors should work:
> Vector3(1, Vector2(2, 3)) == Vector3(1, 2, 3)

We don't need this because we have star args:

Vector3(1, *Vector2(2, 3))

Yeah, good point.

There is this error message for this case at least:
    >>> Vector3(1, (1,2))
    ValueError: Vector3 must be initialized with 3 real numbers or a sequence of 3 real numbers

I don't think the "Lower dimensional to higher dimension constructors should work" bit is that important.
It's not super common in GLSL code I've seen in the wild anyway.

And as you say, we have * notation for this.





Some more benchmarking.

Some timing on python 3.6 (with PyArg_ParseTupleAndKeywords which supports keyword arguments).
Note v3.xyz is the fastest constructor. See below with just PyArg_ParseTuple (no speed improvement).


v3 = Vector3(0)

%timeit v3.xyz
10000000 loops, best of 3: 83.3 ns per loop

%timeit Vector3()
10000000 loops, best of 3: 131 ns per loop

%timeit Vector3(0)
1000000 loops, best of 3: 199 ns per loop

%timeit Vector3(0, 0, 0)
1000000 loops, best of 3: 269 ns per loop

%timeit Vector3(x=0, y=0, z=0)
1000000 loops, best of 3: 426 ns per loop

%timeit Rect((0, 0, 0, 0))
10000000 loops, best of 3: 184 ns per loop

from numpy import array
%timeit array((0,0,0))
1000000 loops, best of 3: 919 ns per loop

assert id(v3) != id(v3.xyz), ".xyz makes a copy"

%timeit (0,0,0)
100000000 loops, best of 3: 14.9 ns per loop

somevec= (1, 2)

%timeit Vector3(1, *somevec)
1000000 loops, best of 3: 333 ns per loop

%timeit anothervec=v3.xyz;anothervec.xyz = (1, 2, 3)
1000000 loops, best of 3: 221 ns per loop





Now timing with PyArg_ParseTuple.
Interestingly, in 3.6 PyArg_ParseTupleAndKeywords and PyArg_ParseTuple
are the same speed (on this mac laptop anyway).

%timeit Vector3(0)
1000000 loops, best of 3: 198 ns per loop

%timeit Vector3(0, 0, 0)
1000000 loops, best of 3: 276 ns per loop


Exactly the same timing as PyArg_ParseTupleAndKeywords pretty much.


And now for something kind of funny...

In [11]: from dis import dis

In [12]: def a():
    ...:     Vector3(0,0,0)
    ...:    

In [13]: def b():
    ...:     v3.xyz
    ...:    

In [14]: dis(a)
  2           0 LOAD_GLOBAL              0 (Vector3)
              2 LOAD_CONST               1 (0)
              4 LOAD_CONST               1 (0)
              6 LOAD_CONST               1 (0)
              8 CALL_FUNCTION            3
             10 POP_TOP
             12 LOAD_CONST               0 (None)
             14 RETURN_VALUE

In [15]: dis(b)
  2           0 LOAD_GLOBAL              0 (v3)
              2 LOAD_ATTR                1 (xyz)
              4 POP_TOP
              6 LOAD_CONST               0 (None)
              8 RETURN_VALUE

An ugly hack would be to
  avoid those byte code ops,
  and avoid PyArg_ParseTupleX in CALL_FUNCTION...

And encode numbers in a swizzle-ish attribute.
  Vector3.xyz_1234234235__23423__333 -> Vector3(1234234235, 23423, 333)

Haha... pretty ridiculous. Not a good idea for readability.
But interesting how you can abuse the attribute getter.


It's sort of unfair that tuple construction gets a 18x faster constructor over extension types in CPython.
But anyway. 3.4x to 11x faster construction than numpy arrays is nice.