There are two issues with your code:
- you do not consider lenght-1 subsequences. You should add
e in the sums list.
- your nested loop starts from the index at which the first occurrence of
e is observed and not the actual index of e. This is a problem in case of duplicates.
Hence, the following should work:
def maxSequence_fixed(arr):
sums = []
for j, e in enumerate(arr):
sums.append(e) # <-- HERE
for i in range(j + 1, len(arr)):
e += arr[i]
sums.append(e)
if all(i<0 for i in arr) or not sums:
return 0
else:
max_sum = max(sums)
return max_sum
Note that there are more efficient / cleaner approaches than this, e.g.:
def max_sum_subseq(items):
iter_items = iter(items)
try:
temp_sum = next(iter_items)
except StopIteration:
temp_sum = 0
max_sum = temp_sum
for item in iter_items:
temp_sum = max(temp_sum + item, item)
max_sum = max(temp_sum, max_sum)
return max(max_sum, 0)
which is O(N) in time and O(1) in memory.
Note that the case of all-negative inputs is handled by max(max_sum, 0) just before return. In case that is not required, it is sufficient to return just max_sum.
An alternative approach would have been to use max(next(iter_items), 0) instead of just next(iter_items) as the first temp_sum value.
And a faster approach would avoid the relatively expensive max() calls:
def max_sum_subseq_fast(items):
iter_items = iter(items)
try:
temp_sum = next(iter_items)
except StopIteration:
temp_sum = 0
max_sum = temp_sum
for item in iter_items:
temp_sum += item
if item > temp_sum:
temp_sum = item
if temp_sum > max_sum:
max_sum = temp_sum
return max_sum if max_sum > 0 else 0
Note that both max_sum_subseq() and max_sum_subseq_fast() can work on any Iterable (and do not require a Sequence, contrarily to the other solutions compared).
If one need to support only Sequences, then the block:
...
iter_items = iter(items)
try:
temp_sum = next(iter_items)
except StopIteration:
temp_sum = 0
max_sum = temp_sum
for item in iter_items:
...
can be replaced with:
...
if len(items) < 1:
return 0
max_sum = temp_sum = 0
for item in items:
...
with only very marginal O(1) speed gain.
Another, much less efficient (but closer to your thinking) solution would be:
def max_sum_subseq_slow(items):
max_sum = 0
for i in range(len(items)):
for j in range(i, len(items)):
temp_sum = sum(items[i:j + 1])
if temp_sum > max_sum:
max_sum = temp_sum
return max_sum
Some sanity checks below:
seqs = [
[],
[-1],
[1, -1],
[1, -1, 1],
[1, -1, 1, 1],
[1, -1, 1, 1, 1],
[1, -1, -1, 1, 1],
]
funcs = maxSequence_OP, maxSequence_fixed, max_sum_subseq, max_sum_subseq_fast, max_sum_subseq_slow
for seq in seqs:
print(seq)
for func in funcs:
print(func(seq))
# []
# 0
# 0
# 0
# 0
# 0
# [-1]
# 0
# 0
# 0
# 0
# 0
# [1, -1]
# 0
# 1
# 1
# 1
# 1
# [1, -1, 1]
# 1
# 1
# 1
# 1
# 1
# [1, -1, 1, 1]
# 2
# 2
# 2
# 2
# 2
# [1, -1, 1, 1, 1]
# 3
# 3
# 3
# 3
# 3
# [1, -1, -1, 1, 1]
# 1
# 2
# 2
# 2
# 2
More test cases can be generate using:
import random
seqs = [[random.randint(-10, 10) for _ in range(20)] for __ in range(100)]
Some timings:
funcs = maxSequence_OP, maxSequence_fixed, max_sum_subseq, max_sum_subseq_fast, max_sum_subseq_slow, maxSequence_Alexander
seq = [random.randint(-10, 10) for _ in range(1000)]
for func in funcs:
print()
print(func.__name__)
%timeit func(seq)
# maxSequence_OP
# 10 loops, best of 3: 168 ms per loop
# maxSequence_fixed
# 10 loops, best of 3: 78 ms per loop
# max_sum_subseq
# 1000 loops, best of 3: 292 µs per loop
# max_sum_subseq_fast
# 10000 loops, best of 3: 66.3 µs per loop
# max_sum_subseq_slow
# 1 loop, best of 3: 1.21 s per loop
# maxSequence_Alexander
# 10000 loops, best of 3: 183 µs per loop
(EDITED to fix minor issues, include a faster solution and updated benchmarks, including a comparison with @Alexander's solution)
index(e). For example:for idx, val in enumerate(arr):