karpathy
backward member implementation question
Why can't this function simply be implemented as follows? Am I missing something? We are dealing with a composite structure.
def backward(self, is_first=True):
if (is_first == True):
self.grad = 1.0
self._backward()
for c in self._prev:
c.backward(False)
The current implementation first does a reverse topological sort on all nodes (No parent comes before its child. For example x and y are parents of the expression (x+y). ) Then it calls _backward() in that order. So when we call the function _backward() for a node, we guarantee that its .grad property has its final (and correct) value and will not change in the future.
Consider the following example:
- a = 2
- b = 3
- c = a + b
- d = a * b
- e = c + d
- f = c * d
- y = e + f
Your method will give an incorrect result for this example. I won't show it step by step as it looks too long and complex in text but you can calculate it with hand to see the problem. The problem will occur in the following way:
- You set y.grad = 1 (correct)
- You set e.grad = 1 (correct)
- Then, because you are using recursion, you will go into the node "c".
- You will set c.grad = 1 (This is not the complete/correct result obviously. This is only the part that comes from e. You will also need to add a +d term when you are calculating the gradient through f. So, as you can verify via calculus dy/dc = 1+d)
- Now, again using recursion you will continue to the "a" node.
- Now you will set a.grad = 1 (This is incorrect, you have calculated it thinking that dy/dc = 1. But the correct value was 1+d.)
I hope I could explain the problem clearly. The problem with your method is basically using the wrong, temporary .grad values of a node to calculate the .grad of its children.
If you are confused by the whole node and graph logic, watching this video would help you understand the structure more clearly as @karpathy imagined it.
