Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix configuration example #1454

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ammar-elsabe
Copy link

@ammar-elsabe ammar-elsabe commented Mar 6, 2023

Description

The configuration example in the documentation provides keys different from what the code at train_onpolicy.py uses, causing a KeyError.

The configuration example for the backtest also uses a model checkpoint path different from what is generated by the training script ./checkpoints/latest.pth vs ./checkpoints/checkpoints/latest.pth

Further, the backtesting script ran into issues because of this update to tianshou which now passes info in PG-based policies, this affects order execution as a whole in qlib as that is not used in this implementation (to the best of my knowledge). Adding a parameter to set info to None in the batch fixes this.

Motivation and Context

Running the RL example, whether in the notebook or the scripts provided, did not work until these changes were implemented, after which they ran and the backtesting results were outputted successfully

How Has This Been Tested?

Changes applied locally and scripts ran successfully
Notebook:
image
Example YAML configurations:
image

Types of changes

  • Fix bugs
  • Add new feature
  • Update documentation

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Mar 6, 2023
@github-actions github-actions bot added the waiting for triage Cannot auto-triage, wait for triage. label Mar 6, 2023
@ammar-elsabe
Copy link
Author

ammar-elsabe commented Mar 6, 2023 via email

AttributeError: 'dict' object has no attribute 'info'
@@ -323,7 +323,7 @@
"simulator = SimpleSimulator(100.0, NSTEPS)\n",
"state = simulator.get_state()\n",
"obs = [{\"obs\": state_interpreter.interpret(state)}]\n",
"policy_out = policy(Batch(obs))\n",
"policy_out = policy(Batch(obs, info=None))\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

I ran the script simple_example.ipynb without encountering any errors. Could you please provide me with instructions on how to reproduce the issue you mentioned? (My tianshou version is 0.4.10)

Copy link
Author

Choose a reason for hiding this comment

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

This was a very long time ago, since then all the RL examples save for this notebook have been deleted from the repository, and it may have been fixed since. I recall I had the latest tianshou version at the time (0.4.11) and just ran the script before realizing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation waiting for triage Cannot auto-triage, wait for triage.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants