<p>Yeah, it seems like it's a wrong implementation.
vals_ref_v is a matrix of 1 row, and 128 columns.
value_v.detach() is a matrix of 128 row </p>

CC BY-SA 4.0
2020-05-16T01:04:00.050Z
https://github.com/PacktPublishing/Deep-Reinforcement-Learning-Hands-On/issues/74
jgauth
CC BY-SA 4.0
2020-05-16T00:50:41.687Z
2020-05-16T00:50:41.687Z
C Lu
<p>I changed the line <code>adv_v = vals_ref_v - value_v.detach()</code> to <code>adv_v = vals_ref_v - value_v.squeeze(-1).detach()</code>. It seems the convergence is much faster. According to the A2C algorithm, it is just logic to apply <span class="math-container">$Q(a, s) - V(s)$</span>, where <span class="math-container">$Q(a, s)$</span> and <span class="math-container">$V(s)$</span> with the same shape.  </p>

<p>The call to <code>detach()</code> is important here as we don't want to propagate the PG into our value approximation head.</p>

CC BY-SA 4.0
2020-05-15T20:27:37.550Z
2020-05-15T20:27:37.550Z
jgauth
<p>Here is the <a href="https://github.com/PacktPublishing/Deep-Reinforcement-Learning-Hands-On/blob/master/Chapter10/02_pong_a2c.py" rel="nofollow noreferrer">code</a> written by Maxim Lapan. I am reading his book (<em>Deep Reinforcement Learning Hands-on</em>). I have seen a line in his code which is really weird. In the accumulation of the policy gradient <span class="math-container">$$\partial \theta_{\pi} \gets \partial \theta_{\pi} + \nabla_{\theta}\log\pi_{\theta} (a_i | s_i) (R - V_{\theta}(s_i))$$</span> we have to compute the advantage <span class="math-container">$R - V_{\theta}(s_i)$</span>. In line 138, maxim uses <code>adv_v = vals_ref_v - value_v.detach()</code>. Visually, it looks fine, but look at the shape of each term. </p>

<pre><code>ipdb&gt; adv_v.shape                                                                                                                            
torch.Size([128, 128])

ipdb&gt; vals_ref_v.shape                                                                                                                       
torch.Size([128])

ipdb&gt; values_v.detach().shape                                                                                                                
torch.Size([128, 1]) 
</code></pre>

<p>In a much simpler code, it is equivalent to </p>

<pre><code>In [1]: import torch                                                            

In [2]: t1 = torch.tensor([1, 2, 3])                                            

In [3]: t2 = torch.tensor([[4], [5], [6]])                                      

In [4]: t1 - t2                                                                 
Out[4]: 
tensor([[-3, -2, -1],
        [-4, -3, -2],
        [-5, -4, -3]])

In [5]: t1 - t2.detach()                                                        
Out[5]: 
tensor([[-3, -2, -1],
        [-4, -3, -2],
        [-5, -4, -3]])
</code></pre>

<p>I have trained the agent with his code and it works perfectly fine. I am very confused why it is good practice and what it is doing. Could someone enlighten me on the line <code>adv_v = vals_ref_v - value_v.detach()</code>? For me, the right thing to do was <code>adv_v = vals_ref_v - value_v.squeeze(-1)</code>.</p>

<p>Here is the full algorithm used in his book : </p>

<p><strong>UPDATE</strong> </p>

<p><a href="https://i.sstatic.net/aNOfJ.png" rel="nofollow noreferrer"><img src="https://i.sstatic.net/aNOfJ.png" alt="enter image description here"></a></p>

<p>As you can see by the image, it is converging even though <code>adv_v = vals_ref_v - value_v.detach()</code> looks wrongly implemented. It is not done yet, but I will update the question later.</p>

CC BY-SA 4.0
2020-05-15T07:04:52.317Z
Could you clarify, you trained the agent with his code or with your modified code? If both, did you notice any difference in behaviour?
dan888
CC BY-SA 4.0
2020-05-15T11:20:33.167Z
@dan888 With his code. I just cloned the repo.
jgauth
CC BY-SA 4.0
2020-05-15T13:04:32.703Z
That's interesting. I would say that what you are doing would make more sense as well. I noticed you created an issue on github, I'm curious to see what comes out of that.
dan888
CC BY-SA 4.0
2020-05-15T13:22:53.077Z
He used the same line many times in his book, but I can't say why it is working and what it is doing exactly. I am going to update the question in few hours to add a graph of what the mean rewards look like. In his book, maxim said *The call to `value_v.detach()` is important, as we don't want to propagate the PG into our value approximation head*. That didn't help me, but maybe it will help someone.
jgauth
<python><policy-gradients><pytorch><actor-critic-methods>
Advantage computed the wrong way?
jgauth
# Advantage computed the wrong way?

(Asked by: username_0 on 2020-05-14)

Here is the code written by Maxim Lapan. I am reading his book (Deep Reinforcement Learning Hands-on). I have seen a line in his code which is really weird. In the accumulation of the policy gradient $$\partial \theta_{\pi} \gets \partial \theta_{\pi} + \nabla_{\theta}\log\pi_{\theta} (a_i | s_i) (R - V_{\theta}(s_i))$$ we have to compute the advantage $R - V_{\theta}(s_i)$. In line 138, maxim uses adv_v = vals_ref_v - value_v.detach(). Visually, it looks fine, but look at the shape of each term. 
ipdb> adv_v.shape                                                                                                                            
torch.Size([128, 128])

ipdb> vals_ref_v.shape                                                                                                                       
torch.Size([128])

ipdb> values_v.detach().shape                                                                                                                
torch.Size([128, 1]) 

In a much simpler code, it is equivalent to 
In [1]: import torch                                                            

In [2]: t1 = torch.tensor([1, 2, 3])                                            

In [3]: t2 = torch.tensor([[4], [5], [6]])                                      

In [4]: t1 - t2                                                                 
Out[4]: 
tensor([[-3, -2, -1],
        [-4, -3, -2],
        [-5, -4, -3]])

In [5]: t1 - t2.detach()                                                        
Out[5]: 
tensor([[-3, -2, -1],
        [-4, -3, -2],
        [-5, -4, -3]])

I have trained the agent with his code and it works perfectly fine. I am very confused why it is good practice and what it is doing. Could someone enlighten me on the line adv_v = vals_ref_v - value_v.detach()? For me, the right thing to do was adv_v = vals_ref_v - value_v.squeeze(-1).
Here is the full algorithm used in his book : 
UPDATE 

As you can see by the image, it is converging even though adv_v = vals_ref_v - value_v.detach() looks wrongly implemented. It is not done yet, but I will update the question later.

**Tags:** python, policy-gradients, pytorch, actor-critic-methods

> **Comments:**
> **username_1**: Could you clarify, you trained the agent with his code or with your modified code? If both, did you notice any difference in behaviour?  
> **username_0**: @username_1 With his code. I just cloned the repo.  
> **username_1**: That's interesting. I would say that what you are doing would make more sense as well. I noticed you created an issue on github, I'm curious to see what comes out of that.  
> **username_0**: He used the same line many times in his book, but I can't say why it is working and what it is doing exactly. I am going to update the question in few hours to add a graph of what the mean rewards look like. In his book, maxim said *The call to `value_v.detach()` is important, as we don't want to propagate the PG into our value approximation head*. That didn't help me, but maybe it will help someone.  

---

## 2 answers

### Answer by username_2 on 2020-05-16. Score: 2

Yeah, it seems like it's a wrong implementation.
vals\_ref\_v is a matrix of 1 row, and 128 columns.
value\_v.detach() is a matrix of 128 row

> **Comments:**
> **username_0**: https://github.com/PacktPublishing/Deep-Reinforcement-Learning-Hands-On/issues/74  

---

### Answer by username_0 on 2020-05-15. Score: 1

I changed the line `adv_v = vals_ref_v - value_v.detach()` to `adv_v = vals_ref_v - value_v.squeeze(-1).detach()`. It seems the convergence is much faster. According to the A2C algorithm, it is just logic to apply $Q(a, s) - V(s)$, where $Q(a, s)$ and $V(s)$ with the same shape.

The call to `detach()` is important here as we don't want to propagate the PG into our value approximation head.


---

