[Docs] Fix multi-agent tutorial #1599
Conversation
Signed-off-by: Matteo Bettini <[email protected]>
facebook-github-bot
added
the
CLA Signed
Signed-off-by: Matteo Bettini <[email protected]>
Signed-off-by: Matteo Bettini <[email protected]>
Signed-off-by: Matteo Bettini <[email protected]>
| for tensordict_data in collector: | ||
| tensordict_data.set( | ||
| ("next", "done"), | ||
| ("next", "agents", "done"), | ||
| tensordict_data.get(("next", "done")) | ||
| .unsqueeze(-1) | ||
| .expand(tensordict_data.get(("next", env.reward_key)).shape), | ||
| ) # We need to expand the done to match the reward shape (this is expected by the value estimator) | ||
| .expand(tensordict_data.get_item_shape(("next", env.reward_key))), | ||
| ) | ||
| tensordict_data.set( | ||
| ("next", "agents", "terminated"), | ||
| tensordict_data.get(("next", "terminated")) | ||
| .unsqueeze(-1) | ||
| .expand(tensordict_data.get_item_shape(("next", env.reward_key))), | ||
| ) | ||
| # We need to expand the done and terminated to match the reward shape (this is expected by the value estimator) |
There was a problem hiding this comment.
what is the best way to approach this?
A common criticism of our tutorials is that they are too complex, I wonder what will be judged as the most complex thing here: (1) boilerplate code such as this, which IMO has little instructive value or (2) an ad-hoc transform that may be a bit too custom to be part of the core lib
There was a problem hiding this comment.
Yeah i agree, what i can do is:
- leave as is
- make the transform in the tutorial
- make the transform in core
- We could also consider allowing rewards and dones with different shapes and expanding in the value functionals
What is your preference ?
There was a problem hiding this comment.
- seems like a stretch, these classes are already very complex and hard to maintain
I think hese operations are actually 2fold: (1) rename without deleting and (2) unsqueeze + expand
We have a transform that does rename, one that does unsqueeze. We just need one that does expand (or expand_as).
The quickest way forward is this:
Merge as is, open an issue with a feature request for the expand_as transform. Wdyt?
Signed-off-by: Matteo Bettini <[email protected]>
Signed-off-by: Matteo Bettini <[email protected]>
Fix the multi-agent tutorial after the recent "termianted" introduction