[Docs] Clarify DQN hiddens docstring as dueling-only (#43051)#63444
[Docs] Clarify DQN hiddens docstring as dueling-only (#43051)#63444dstrodtman wants to merge 1 commit into
Conversation
The hiddens parameter in rllib/algorithms/dqn/dqn.py only applies to the advantage and value branches of a dueling DQN. Append "in a dueling architecture" to the docstring so users do not confuse it with model.fcnet_hiddens. Closes ray-project#43051 [DOC-981] Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Douglas Strodtman <douglas@anyscale.com>
There was a problem hiding this comment.
Code Review
This pull request updates the docstring for the hiddens parameter in the DQN training method. The reviewer noted that the new description is misleading because the parameter is used regardless of whether a dueling architecture is enabled. Furthermore, the feedback identifies a typo in the docstring and an incorrect type hint in the method signature that requires a fix.
| dueling: Whether to use dueling DQN. | ||
| hiddens: Dense-layer setup for each the advantage branch and the value | ||
| branch | ||
| branch in a dueling architecture. |
There was a problem hiding this comment.
The addition "in a dueling architecture" is technically misleading. In the current DQN implementation (both for PyTorch and TensorFlow), the hiddens parameter is also used to define the hidden layers of the single Q-network head when dueling=False. Specifying it as dueling-only might lead users to believe the parameter is ignored in non-dueling configurations. Additionally, there is a typo in the preceding line (307): "each the" should be "each of the" or just "the". Finally, please note that the type hint for hiddens in the training method signature (line 231) is Optional[int], but it should be Optional[List[int]] to match its usage and initialization.
Description
The
hiddensparameter inrllib/algorithms/dqn/dqn.pyonly applies to the advantage and value branches of a dueling DQN, but the docstring did not say so. As a result, users could confuse it withmodel.fcnet_hiddens(see linked issue). This PR appends "in a dueling architecture" to the docstring to make the scope explicit.Related issues
Closes #43051
[DOC-981]
Additional information
Target file:
rllib/algorithms/dqn/dqn.py(thehiddensentry in thetrainingargument list).Generated with Claude Code