Below is my solution to LC #494: Target Sum (given a list of numbers and a target integer, find out how many different ways the numbers can be subtracted from the total sum to result in the target sum).
There's one very common pattern in Python here that I question, and it's the use of for num in nums[1:]:
. Iterating over nums[1:]
is very explicit and readable, but it results in the creation of a whole new array when that really isn't our intention. That also means additional wasted time and wasted memory.
I used to always do for i in range(1, len(nums)):
in these situations, but I see most other people using nums[1:]
. Would love to hear thoughts on that.
And if there's a cleaner way to write this without having to initialize the counts
hash from nums[0]
separately from the body of the loop, that'd be great too. I couldn't think of a cleaner way.
class Solution:
def findTargetSumWays(self, nums: List[int], S: int) -> int:
counts = {nums[0]: 1, -nums[0]: 1}
if nums[0] == 0:
counts[0] += 1
for num in nums[1:]:
keys = list(counts)
new = {}
for key in keys:
if key + num not in new:
new[key + num] = 0
if key - num not in new:
new[key - num] = 0
new[key + num] += counts[key]
new[key - num] += counts[key]
counts = new
return counts[S] if S in counts else 0
```
2 Answers 2
for num in nums[1:]
is the prefered way to iterate over a partition of an array; if you are worried about performance, it actually is faster, because accessing by index is more expensive in python than copying then iterating values (you can play with snippets of timeit to verify that claim, but I'm fairly sure of that - accessing by index in python involves way more checks and operations than it does in a language like C).
The performance issue with your program, if there is one, isn't there. The copy is just done once in the outter loop, and since it is repeated by num your inner loop is way more costy. Mind this: since every iteration can in the worst case double the number of items in keys
, your program is actually of complexity O(2^n)
where n is the array length.
In a matter of using more efficient structures you can also use Counter
from collections
that saves a bit of verbosity (it has zero as default value) and computation time (it is optimized C structure).
Without optimizing the algorithm, which I believe is part of the challenge :
from collections import Counter
class Solution:
def findTargetSumWays(self, nums: List[int], S: int) -> int:
counts = Counter()
counts[-nums[0]] += 1
counts[nums[0]] += 1
for num in nums[1:]:
new = Counter()
for key in counts.keys():
new[key + num] += counts[key]
new[key - num] += counts[key]
counts = new
return counts[S]
-
\$\begingroup\$ Your base solution is not bad, adding a small optimization I was able to best 87.82% of submitted solutions in run time. \$\endgroup\$Diane M– Diane M2019年08月31日 00:15:24 +00:00Commented Aug 31, 2019 at 0:15
-
\$\begingroup\$ Can I ask what your optimization was? Also, what do you think about using
collections.defaultdict(int)
instead ofcollections.Counter
(since we're only using the default value feature)? \$\endgroup\$NightDriveDrones– NightDriveDrones2019年08月31日 12:54:29 +00:00Commented Aug 31, 2019 at 12:54 -
\$\begingroup\$ @user107870 I believe Counter is more optimized because it's less generic but I have no idea if it really is. The optimization was 1- sort nums in descending order and 2- remove keys in counts if they are distant of S of more than the sum of remaining nums. \$\endgroup\$Diane M– Diane M2019年08月31日 18:26:42 +00:00Commented Aug 31, 2019 at 18:26
List objects are iterables. A for
loop implicitly calls iter()
on the list, but you can do it yourself, too.
nums_iter = iter(nums)
num0 = next(nums_iter)
for num in nums_iter:
# ...
The list is not copied, and the for
loop will begin with the second item, as required.
However, this is far less clear than for num in nums[1:]:
Much clearer is to leverage itertools
, specifically itertools.islice()
:
for num in islice(nums, 1, None):
# ...
However, islice()
also allows a step
greater than one, which means you probably lose performance to the code that supports this unneeded capability.
Copying the array might be a smaller performance penalty. You’ll want to profile each approach, and evaluate the trade off between readability and performance.
Speaking of unnecessary copies:
keys = list(counts)
counts
is not changed in the loop below, so memorizing the list of keys is an unnecessary copy. You could simply use:
for key in counts:
# ...
counts[key]
is an unnecessary lookup. And you are doing it twice. Better is to retrieve the value with the key during the iteration process.
for key, val in counts.items():
# ...
And, as mentioned in @ArthurHavlicek’s answer, use collections.Counter
.
-
\$\begingroup\$ After reading docs, I can see
islice
might be useful if we were using a single iterator in multiple contexts, but is it adding anything overrange
/xrange
when we're just doing a single loop through? \$\endgroup\$NightDriveDrones– NightDriveDrones2019年08月31日 12:38:03 +00:00Commented Aug 31, 2019 at 12:38 -
\$\begingroup\$ The documentation says "Roughly equivalent to", and then a blob of Python code. Don’t get fooled; it is not implemented in Python. It will be implemented directly in C, (assuming CPython), and may approach the performance of looping over the list directly, but without the penalty of copying the list. \$\endgroup\$AJNeufeld– AJNeufeld2019年09月01日 15:51:27 +00:00Commented Sep 1, 2019 at 15:51
Explore related questions
See similar questions with these tags.