Skip to main content
Code Review

Return to Answer

added 842 characters in body
Source Link
Graipher
Graipher
  • 41.1k
  • 7
  • 69
  • 133

You can get rid of your duplications by using loops. This comes without any performance hit in this case. Again, the limits have to be defined outside of the function, but this function can deal with a variable number of arguments, just like the numpy solution below.

limits = amax, bmax, cmax, dmax


def get_check_loop(*args):
    # assert len(limits) == len(args)
    for x, limit in zip(args, limits):
        if x == 0:
            i = len(args) - 1
            for t in reversed(args):
                if t - limit < 0:
                    return i
                i -= 1
    return -1

I had to use the slightly ugly manual decreasing of the inner loop variable, because enumerate is not an iterable, at least not as understood by reversed.

Ok, I found a numpy solution for this. It is slower than your approach, but shorter. Specifically, it is about 50 times slower (~10\$\mu\$s instead of ~200ns per call, on my machine).

Ok, I found a numpy solution for this. It is slower than your approach, but shorter. Specifically, it is about 50 times slower (~10\$\mu\$s instead of ~200ns per call, on my machine).

You can get rid of your duplications by using loops. This comes without any performance hit in this case. Again, the limits have to be defined outside of the function, but this function can deal with a variable number of arguments, just like the numpy solution below.

limits = amax, bmax, cmax, dmax


def get_check_loop(*args):
    # assert len(limits) == len(args)
    for x, limit in zip(args, limits):
        if x == 0:
            i = len(args) - 1
            for t in reversed(args):
                if t - limit < 0:
                    return i
                i -= 1
    return -1

I had to use the slightly ugly manual decreasing of the inner loop variable, because enumerate is not an iterable, at least not as understood by reversed.

Ok, I found a numpy solution for this. It is slower than your approach, but shorter. Specifically, it is about 50 times slower (~10\$\mu\$s instead of ~200ns per call, on my machine).

Source Link
Graipher
Graipher
  • 41.1k
  • 7
  • 69
  • 133

Ok, I found a numpy solution for this. It is slower than your approach, but shorter. Specifically, it is about 50 times slower (~10\$\mu\$s instead of ~200ns per call, on my machine).

First, I took your code and put it into a function. I left the max values outside of it. (This makes it slightly slower, because looking up global variables is slower. On the other hand, re-defining variables every call is also costly. In the end the two seem to cancel out.) I also changed the code to return numeric values (because that is easier for the numpy code I wrote). You can always use it as an index into the list ["a", "b", "c", "d", None], where the last entry is for the case that none of the values is zero, which I also included:

amax = 500
bmax = 1000
cmax = 2000
dmax = 10000


def get_check(a, b, c, d):
    if a == 0:
        if d - amax < 0:
            return 3
        elif c - amax < 0:
            return 2
        elif b - amax < 0:
            return 1
        else:
            return 0
    elif b == 0:
        if d - bmax < 0:
            return 3
        elif c - bmax < 0:
            return 2
        else:
            return 1
    elif c == 0:
        if d - cmax < 0:
            return 3
        else:
            return 2
    elif d == 0:
        return 3
    else:
        return -1

For the numpy approach we need to realize that if one of the variables is zero, then we need to subtract that variables max interval from all arguments. For example, if x = a, b, c, d = 351, 0, 304, 1500, then we need to have a look at x - bmax < 0 and choose the index of the last one that is below zero.

In code this is:

import numpy as np

bounds = np.array([500, 1000, 2000, 10000])


def argmax_last(x):
    """
    Returns the last occurrence of the maximum value in x.

    From http://stackoverflow.com/a/8768734/4042267
    """
    return len(x) - np.argmax(x[::-1]) - 1


def get_check_np(*v):
    if any(x == 0 for x in v):
        return argmax_last(v - bounds[np.argmin(v)] < 0)
    return -1

np.argmax (np.argmin) returns the index of the first maximum (minimum) in the passed array. argmax_last returns the last index where the value is the maximal value in that array. Examples:

>>> max([4, 1, 2, 4, 3, 2])
4
>>> np.argmax([4, 1, 2, 4, 3, 2])
0
>>> argmax_last([4, 1, 2, 4, 3, 2])
3

Note that the numpy function is more easily extendable (it only cares that bounds is at least as long as the position of the zero in the input arguments).