12
\$\begingroup\$

The code consists of three functions:

  1. Spans arbitrary vector (takes dimension as an argument)

  2. Spans vector orthogonal to the one passed in the argument

  3. Finds cross product between two vectors.

The code is following:

def span_vector(n):
    '''n represents dimension of the vector.'''
    return [random.randrange(-1000,1000) for x in range(n)]


def span_orthogonal(vec):
    '''vec represents n-dimensional vector'''
    '''Function spans an arbitrary vector that is orthogonal to vec'''

    dimension = len(vec)
    orthogonal_vec = []
    for k in range(dimension-1):
        orthogonal_vec.append(random.randrange(-1000,1000))
    last_k = ((-1)*sum([vec[x]*orthogonal_vec[x] for x in range(dimension-1)]))/vec[-1]
    orthogonal_vec.append(last_k)
    return orthogonal_vec

def cross_product(v1,v2):
    return sum(v1[x]*v2[x] for x in range(len(v1)))

What can be improved?


EDIT The last function must read dot_product, but not cross_product. I made a mistake.

\$\endgroup\$
  • 2
    \$\begingroup\$ Is there a reason that you aren't using Numpy? \$\endgroup\$ – Reinderien Sep 18 at 13:42
  • \$\begingroup\$ @Reinderien I've just tried to write something without using Numpy. But well, I concede that using Numpy would be a way better choice. \$\endgroup\$ – Nelver Sep 18 at 14:27
  • 8
    \$\begingroup\$ Two points: (1) "span" has a technical meaning in linear algebra so I do not think the names span_vector and span_orthogonal are appropriate (sth. like generate_vector should be fine); (2) if it is not required that span_orthogonal returns a random vector, one can directly construct a vector orthogonal to the input from the input's coordinates. \$\endgroup\$ – GZ0 Sep 18 at 14:41
11
\$\begingroup\$

You should probably be using Numpy, although I don't know enough about your situation to comment any further.

Assuming that you need to retain "pure Python", the following improvements can be made:

Negation

Replace (-1)* with -

Generators

Replace your for k in range(dimension-1): loop with

orthogonal_vec = [
    random.randrange(-1000,1000)
    for _ in range(dimension-1)
]

Type hints

n: int, vec: typing.Sequence[float] (probably) . And the first two functions return -> typing.List[float]. cross_product both accepts and returns float.

Inner list

sum([ ... ])

shouldn't use an inner list. Just pass the generator directly to sum.

\$\endgroup\$
10
\$\begingroup\$

This is only a minor observation on top of what @Reinderien already wrote about your code.

Writing function documentation like you did with

def span_orthogonal(vec):
    '''vec represents n-dimensional vector'''
    '''Function spans an arbitrary vector that is orthogonal to vec'''

does not work as expected.

If you were to use help(span_orthogonal) you'd see

Help on function span_orthogonal in module __main__:

span_orthogonal(vec)
    vec represents n-dimensional vector

The reason is that only the first block of text is interpreted as documentation. Also the usual convention is to write documentation "the other way round", by which I mean first give a short summary on what your function does, than go on to provide details such as the expected input. Both aspects can also be found in the infamous official Style Guide for Python Code (aka PEP 8) in the section on documentation strings.

With

def span_orthogonal(vec):
    '''Function spans an arbitrary vector that is orthogonal to vec

    vec represents n-dimensional vector
    '''

calling help(...) gives you

Help on function span_orthogonal in module __main__:

span_orthogonal(vec)
    Function spans an arbitrary vector that is orthogonal to vec

    vec represents n-dimensional vector

Also since @Reinderien also hinted you towards numpy, just let me tell you that there is also "special" documentation convention (aka numpydoc) often used in the scientific Python stack.

An example:

def span_orthogonal(vec):
    '''Function spans an arbitrary vector that is orthogonal to vec

    Parameters
    ----------
    vec : array_like
        represents n-dimensional vector
    '''

This style is closer to what's possible with type hints in current versions of Python, as in that it's more structured. The idea behind numpydoc is to facilitate automated documentation generation using tools like Sphinx, but this goes a little bit beyond what I was trying to convey here.

\$\endgroup\$
  • \$\begingroup\$ That's a very useful information. Thank you! \$\endgroup\$ – Nelver Sep 18 at 14:54
5
\$\begingroup\$

Besides what @Reinderein and @AlexV already mentioned, you could have added the following to your code to deliver a complete runnable example:

at the top:

import random

at he bottom something like:

def main():
    v1 = span_vector(3)
    v2 = span_orthogonal(v1)
    print(v1)
    print(v2)
    print(cross_product(v1,v2))

if __name__ == '__main__':
    main()

For the 1000's (and in -1000) you could use a 'constant':

MAX_COOR_VAL = 1000

The definition of (cross)dot_product(v1,v2) could be made a bit clearer and more consistent with span_orthogonal(vec):

def dot_product(vec1, vec2):

The method span_orthogonal(vec) is not bulletproof, it might result in a ZeroDivisionError exception when vec equals [1,0] and the random creation of orthogonal_vec happens to be [1] (or [2])

\$\endgroup\$
5
\$\begingroup\$

It would be more Pythonic to use zip() in your cross_product(v1, v2) dot_product(v1, v2) function:

    return sum(a * b for a, b in zip(v1, v2))

This iterates over both vectors simultaneously, extracting one component from each, and calling those components a and b respectively ... and multiplying them together and summing them as normal. No need for the "vulgar" for x in range(len(v1)) antipattern.

\$\endgroup\$
2
\$\begingroup\$

Your span_orthogonal(vec) function is doing stuff from your other functions, so rather than rewriting the code, you can just use those functions:

last_k = -dot_product(span_vector(dimension-1),vec[:-1])/vec[-1]

However, your method of giving all but the last coordinate random values, and then calculating the last coordinate's value based on that, gives an error when the sum for the rest of the components. So you should find a nonzero coordinate, exit the function if none such exists, then find the dot product of the remaining coordinates, then check whether that's zero.

try:
    nonzero_index, nonzero_value = next([(i,v) for (i,v) in enumerate(vec) if v)])
except StopIteration:
    print("Vector must be nonzero.")
    return 
orthogonal_vec = span_vector(dimension-1)
reduced_vec =  vec.copy()
reduced_vec.pop(nonzero_index)      
initial_product = -dot_product(orthogonal_vec,reduced_vector)
if initial_product:
     orthogonal_vec.insert(nonzero_index,-initial_product/nonzero_value)
else:
    orthogonal_vec.insert(non_zero_index,0)
return orthogonal_vec
\$\endgroup\$

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.