1
\$\begingroup\$

I am trying to find an optimal way to get the min and max limits from a list of lists provided a multiplier. I've tried two methods and I'm wondering if there is a more optimal solution that scales better in terms of performance when executed multiple times.

import timeit

def get_lower_upper_limits(inps):
    ranges, multipliers = inps
    min_l = 0
    max_l = 0
    for idx, multiplier in enumerate(multipliers):
        min_l += multiplier * (min(ranges[idx]) if multiplier > 0 else max(ranges[idx]))
        max_l += multiplier * (max(ranges[idx]) if multiplier > 0 else min(ranges[idx]))
    return min_l, max_l

def wrapper(func, *args, **kwargs):
    def wrapped():
        return func(*args, **kwargs)
    return wrapped

ranges = [[1000, 1400, 1800], [2200, 2400, 2600], [3000, 3100, 3200]]
multiplier = [-1, 2, 3]
assert len(ranges) == len(multiplier)
wrapped = wrapper(get_lower_upper_limits, (ranges, multiplier))
print('get_lower_upper_limits: {}'.format(timeit.timeit(wrapped, number=5000000)))
   

get_lower_upper_limits: 9.775336363000001

Example:

range = [[1000, 1500, 2000], [2000, 2500, 3000]]

multiplier = [1, 2]

range_after_multiplication = [[1000, 1500, 2000], [4000, 5000, 6000]]

The min/max limit (or number) that can be produced from the sum of each of the elements in the list is:

min_l = 1000 + 4000 = 5000

max_l = 2000 + 6000 = 8000

\$\endgroup\$
3
  • \$\begingroup\$ @superbrain added an example, I hope it's clearer now \$\endgroup\$
    – zeetitan
    Dec 29, 2020 at 0:02
  • 3
    \$\begingroup\$ Seems inefficient to search for min and max in the ranges. They're all sorted, so just take the first and last element. \$\endgroup\$ Dec 29, 2020 at 0:08
  • \$\begingroup\$ this combined with the changes in the answer below help. Thanks \$\endgroup\$
    – zeetitan
    Dec 31, 2020 at 22:16

1 Answer 1

1
\$\begingroup\$

Function Arguments

def get_lower_upper_limits(inps):
    ranges, multipliers = inps
    ...

This is a curious choice for declaring arguments to the function. You must package two arguments together as a tuple for the inps argument, without any clear indication to the caller of the requirement.

Why not declare the function as:

def get_lower_upper_limits(ranges, multipliers):
    ...

The caller then has a clue that the function takes two arguments, a ranges and a multipliers.

A """docstring""" could go a long way towards helping the caller understand the requirements for calling the function. Type-hints would go even further. I recommend researching both subjects.

Wrapper

With the above change to the function arguments, your wrapped function will need a new definition:

wrapped = wrapper(get_lower_upper_limits, (ranges, multiplier))

This presently provides 1 argument (the tuple created with (ranges, multiplier)) to be passed to the function being wrapped. With the above change, the function now needs the ranges and multiplier passed as individual arguments:

wrapped = wrapper(get_lower_upper_limits, ranges, multiplier)

Wrapper-free packaging

The wrapper function is not needed.

First, the partial function from functools will perform the wrapping for you (and provides additional capabilities, which are not needed here). Remove the wrapper function, and replace it with an import:

from functools import partial

wrapped = partial(get_lower_upper_limits, ranges, multiplier)

This may be faster than your hand-written wrapper function, as built-ins can be implemented directly in C.

Again, as this is not using the full capabilities of partial, a simpler alternative exists:

wrapped = lambda: get_lower_upper_limits(ranges, multiplier)

Don't index

In Python, indexing is relatively slow. If you do not need idx, except for use in variable[idx], it is best to loop over variable directly.

Since in this case, you need to loop over two lists ranges and multipliers simultaneously, you'll need to zip them together first (zip as in zipper, not as in compression).

    for values, multiplier in zip(ranges, multipliers):
        min_l += multiplier * (min(values) if multiplier > 0 else max(values))
        ...

Improved code

import timeit

def get_lower_upper_limits(ranges, multipliers):
    min_l = 0
    max_l = 0
    for values, multiplier in zip(ranges, multipliers):
        min_l += multiplier * (min(values) if multiplier > 0 else max(values))
        max_l += multiplier * (max(values) if multiplier > 0 else min(values))
    return min_l, max_l

ranges = [[1000, 1400, 1800], [2200, 2400, 2600], [3000, 3100, 3200]]
multiplier = [-1, 2, 3]
assert len(ranges) == len(multiplier)
func = lambda: get_lower_upper_limits(ranges, multiplier)

print('get_lower_upper_limits: {}'.format(timeit.timeit(func, number=500000)))

Approximately 5% faster

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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