Reputation: 437
I am a Python newbie.
The following was a beginner level problem at codechef.com Link to problem
Problem Text:
Let's consider a triangle of numbers in which a number appears in the first line, two numbers appear in the second line, three in the third line, etc. Develop a program which will compute the largest of the sums of numbers that appear on the paths starting from the top towards the base, so that:
Input
In the first line integer n - the number of test cases (equal to about 1000). Then n test cases follow. Each test case starts with the number of lines which is followed by their content.
Output
For each test case write the determined value in a separate line.
The following is my code:
tempij=[]
s=0
def solve(i,j,ll):
global s
s=0
tempij=[]
if i==len(ll):
return 0
elif (i,j) in tempij:
return s
else:
tempij.append((i,j))
t1=solve(i+1,j,ll)
t2=solve(i+1,j+1,ll)
t=max(t1,t2)+ll[i][j]
s=t
return t
t=int(input())
ll=[]
for k in range(t):
ll=[]
n=int(input())
for i in range(n):
lst=[]
text=input().split()
for j in range(len(text)):
lst.append(int(text[j]))
ll.append(lst)
print(solve(0,0,ll))
Problem with this code:
I have tried to implement Recursion with Memoization in accordance with their editorial on this problem. Link to editorial
Code explained (Relevant part of editorial):
(I am using tempij for caching in the code above. Something similar to SO question: SO question about memoization)
While it works for the test cases given as example, it seems to be exceeding the time limit for other test cases.
My question: How do I improve this code? Or is there a better way?
Upvotes: 0
Views: 749
Reputation: 437
I realized my mistake. Thanks to JCVanHamme.
The above code was not caching correctly because of reasons mentioned by JCVanHamme. I had been resetting the cache after every call. So I did away with all the global variables which were creating problems (since they would have to be re-set for every test case and that was creating a problem). Modified code looks like:
def solve(i,j,ll,cache):
if i==len(ll):
return 0
elif (i,j) not in cache:
t1=solve(i+1,j,ll,cache)
t2=solve(i+1,j+1,ll,cache)
t=max(t1,t2)+ll[i][j]
cache[(i, j)] = t
return cache[(i, j)]
t=int(input())
ll=[]
cache={}
for k in range(t):
ll=[]
cache={}
n=int(input())
for i in range(n):
lst=[]
text=input().split()
for j in range(len(text)):
lst.append(int(text[j]))
ll.append(lst)
print(solve(0,0,ll,cache))
The original question still holds. Can it be made faster and better using python itself? I checked out run-times of people who used other languages like C/C++ and it is as low as 0:08 while for python 3.x it is 0.57!
Upvotes: 0
Reputation: 1050
I'm not convinced your memoization is working. Every time you call solve
, you blow away your cache and you have a local tempij
that shadows the global copy. Moreover, you're only storing one calculated value at a time in s
, so even if it was working, you probably wouldn't retrieve the correct cached value in most cases. Leaving aside the use of the global variable (shudder), I think you should use something more like
cache = {}
def solve(i,j,ll):
global cache
if i==len(ll):
return 0
elif (i,j) not in cache:
t1=solve(i+1,j,ll)
t2=solve(i+1,j+1,ll)
t=max(t1,t2)+ll[i][j]
cache[(i, j)] = t
return cache[(i, j)]
Upvotes: 1