[RLlib] Reinstate trajectory view API tests. #18809
Conversation
| results = trainer.train() | ||
| assert results["train_batch_size"] == config["train_batch_size"] | ||
| assert results["timesteps_total"] == config["train_batch_size"] |
There was a problem hiding this comment.
I have a random question that I have been curious about for a while: how much do we honor the train_batch_size param here?
for example, in complete_episode mode, or if there is sample replay, will we ever give a training batch that is of very different size?
thanks.
There was a problem hiding this comment.
We try to do our best to honor it, but it's not guaranteed to be exact always.
The reason is that we do parallel rollouts with a fixed (or full episode length) step limit per vectorized(!) environment. Depending on the number of vectorized sub-envs per worker and the number of workers, the final train batch may be slightly off. For PPO for example, we auto-correct the rollout_fragment_length (since a few releases ago) based on these factors to better match the train_batch_size, but of course if you have lots of odd numbers in these setting, you will not get the train batch exactly right.
There was a problem hiding this comment.
thanks for the explanation, that's my impression as well.
I actually have a feeling sometimes we may be off a lot.
I can probably do some testing when I get a chance.
thanks.
| SampleBatch.ACTIONS, | ||
| shift=1, | ||
| space=action_space, | ||
| used_for_compute_actions=False) |
There was a problem hiding this comment.
maybe we should have validation for this field somewhere? seems easy to miss, and not straight-forward for regular users.
There was a problem hiding this comment.
You mean to check, whether it's even possible to have this in the action computation event, even though the shift is >0 from a "collected" field, like actions? Great idea!
sven1977
added
the
tests-ok
This PR reinstates the commented-out trajectory view API tests in
rllib/evaluation/tests/test_trajectory_view_api.py.is_recurrent()when used with attention nets;Why are these changes needed?
Related issue number
Checks
scripts/format.shto lint the changes in this PR.