Skip to content

Conversation

@FloWsnr
Copy link
Contributor

@FloWsnr FloWsnr commented May 19, 2025

Hi guys,

I realized that the parameter max_rollout_steps is not present in the dataset docstring nor the docs on the website. This lead to some confusion since we wanted to get a full 500 step trajectory.

Best,
Florian

Copy link
Contributor

@mikemccabe210 mikemccabe210 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for the PR! Also, as a side note - it's an upper limit. If you set it to an arbitrarily large number, you should get the full trajectory.

Copy link
Collaborator

@LTMeyer LTMeyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @FloWsnr for your contribution! Any improvement on the documentation is much welcomed.
I believe it can be even clearer that the full trajectory gets returned in case max_rollout_steps is larger than its actual length.

Co-authored-by: Lucas Meyer <LTMeyer@users.noreply.github.com>
@FloWsnr
Copy link
Contributor Author

FloWsnr commented May 21, 2025

Good idea, @LTMeyer, I commited your change!

@LTMeyer LTMeyer self-requested a review May 21, 2025 14:49
@LTMeyer LTMeyer merged commit cfffc12 into PolymathicAI:master May 21, 2025
kazewong pushed a commit that referenced this pull request Nov 5, 2025
* added max rollout steps to dataset docstring

* Update the_well/data/datasets.py

Co-authored-by: Lucas Meyer <LTMeyer@users.noreply.github.com>

---------

Co-authored-by: Lucas Meyer <LTMeyer@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants