[RLlib; Docs overhaul] Docstring cleanup: Policies, policy_templates. #19759
Conversation
rllib/policy/dynamic_tf_policy.py
Outdated
| @@ -88,57 +72,53 @@ def __init__( | |||
| obs_include_prev_action_reward=DEPRECATED_VALUE): | |||
| """Initializes a DynamicTFPolicy instance. | |||
|
|
|||
| Initialization oxf this class occurs in two phases and defines the | |||
rllib/policy/dynamic_tf_policy.py
Outdated
| action_space: Action space of the policy. | ||
| config: Policy-specific configuration data. | ||
| loss_fn: Function that returns a loss tensor for the policy graph. | ||
| stats_fn: Optional function that returns a dict of TF fetches |
There was a problem hiding this comment.
what does "TF fetches" mean? I thought we can export any numeric stats here?
There was a problem hiding this comment.
Not for TFPolicy. The stats fn here has to return static graph tensors, just like e.g. the loss function.
There was a problem hiding this comment.
added explanation.
rllib/policy/dynamic_tf_policy.py
Outdated
| loss_fn: Function that returns a loss tensor for the policy graph. | ||
| stats_fn: Optional function that returns a dict of TF fetches | ||
| given the policy and batch input tensors. | ||
| grad_stats_fn: Optional function that returns a dict of TF |
There was a problem hiding this comment.
can we explain the difference between stats_fn and grad_stats _fn please?
| generate an action distribution object from, and | ||
| internal-state outputs (or an empty list if not applicable). | ||
| action_distribution_fn: A callable returning distribution inputs | ||
| (parameters), a dist-class to generate an action distribution |
There was a problem hiding this comment.
hmm, personally if I am a new user, I would be quite confused about the relationship between action_sampler_fn and action_distribution_fn.
I think I understand action sampling fn, but what does action distribution fn do?
There was a problem hiding this comment.
Very valid point! I'll try to clarify and also mention that only one of these should be defined.
rllib/policy/tf_policy.py
Outdated
| seq_lens (Optional[TensorType]): Placeholder for RNN sequence | ||
| lengths, of shape [NUM_SEQUENCES]. | ||
| model: The optional ModelV2 to use for calculating actions and | ||
| losses. |
There was a problem hiding this comment.
can we mention what model will get used if not specified?
| """Returns whether the loss term(s) have been initialized.""" | ||
| return len(self._losses) > 0 | ||
|
|
||
| def _initialize_loss(self, losses: List[TensorType], |
There was a problem hiding this comment.
Nope, just moving stuff around to sort methods better by importance. Private/deprecated/helpers move more to the end.
|
Hey @gjoliver thanks for the review. Addressed all requests, could you take another look? |
| generate an action distribution object from, and | ||
| internal-state outputs (or an empty list if not applicable). | ||
| action_distribution_fn: A callable returning distribution inputs | ||
| (parameters), a dist-class to generate an action distribution |
Docstring cleanup: Policies, policy_templates.
Why are these changes needed?
Related issue number
Checks
scripts/format.shto lint the changes in this PR.