Bug in network_buider

[xuanyaoming]

Hi! I think the continue should be replaced with pass here, because continue will skip all the remaining lines in the loop.
If my understanding is correct, dense_func refers to nn.Linear or the dense layer in tenseflow. Then in_size should be updated every time when the loop ends. That means line 109 shouldn't be skipped, otherwise in_size will always be the input size.

By the way, is the last element of units the output size?

Looking forward to your reply.

https://github.com/Denys88/rl_games/blob/811af5c19e9f7ebf7ff0725512bfc64e52439fe8/rl_games/algos_torch/network_builder.py#LL106C26-L106C26

[Denys88]

I think its fine. all other lines a related to the case where we need norm.

[xuanyaoming]

I don't think so. Line 133 is not in the "if-else" codeblock. It runs in all situations. https://github.com/Denys88/rl_games/blob/811af5c19e9f7ebf7ff0725512bfc64e52439fe8/rl_games/algos_torch/network_builder.py#L113C16-L113C16

[Denys88]

Thanks!
But I am surprised now why it works :)

[xuanyaoming]

I guess there are two reasons. First, most people prefer to have the same unit size for all hidden layers. Actually, there is a paper claining doing so can reduce the effect of gradient descent in some situations. Secondly, I think most people (including me) have a habit to normalize internal variables. So the code I'm talking about probably has never been run by anyone yet.

[Denys88]

@xuanyaoming I've got what happened. Btw in the most IsaacGym configs there are like 256-128-64 MLP without normalization.
need_norm is set to true by default and we set it to false only in the case when we want to norm only first layer (I saw this configuration in one of the papers):

                if not need_norm:
                    continue
                if norm_only_first_layer and norm_func_name is not None:
                   need_norm = False 

Thanks for the report I'll fix my code.