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.

Sign up for GitHub

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

Jump to bottom

[Feature] Implicit Q-Learning (IQL) #933

Merged
merged 19 commits into from
Mar 14, 2023
Merged

[Feature] Implicit Q-Learning (IQL) #933

merged 19 commits into from
Mar 14, 2023

Conversation

BY571
Copy link
Contributor

@BY571 BY571 commented Feb 22, 2023 •
edited
Loading

Description

Adding the Implicit Q-Learning (IQL) objective and an online rl example.

Motivation and Context

Adds the first Offline RL Algorithm to TorchRL. However, currently only an online learning example. Converged for the Pendulum-v1 gym environment:

image

With the merge of the offline datasets #928 an extra offline example will be added.

Types of changes

What types of changes does your code introduce? Remove all that do not apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds core functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (update in the documentation)
  • Example (update in the folder of examples)

Checklist

Go over all the following points, and put an x in all the boxes that apply.
If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • I have read the CONTRIBUTION guide (required)
  • My change requires a change to the documentation.
  • I have updated the tests accordingly (required for a bug fix or a new feature).
  • I have updated the documentation accordingly.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 22, 2023
@BY571 BY571 marked this pull request as ready for review February 22, 2023 13:53
vmoens
vmoens approved these changes Feb 24, 2023
View reviewed changes
Copy link
Contributor

@vmoens vmoens left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Can you add the class to the docs?
We should also add the example to the tests (there is one workflow dedicated to these)
Otherwise LGTM, just a few minor comments

torchrl/objectives/iql.py Outdated Show resolved Hide resolved
torchrl/objectives/iql.py Outdated Show resolved Hide resolved
torchrl/objectives/iql.py Outdated Show resolved Hide resolved
examples/iql/iql_online.py Outdated Show resolved Hide resolved
examples/iql/iql_online.py Outdated Show resolved Hide resolved
@vmoens
Copy link
Contributor

vmoens commented Feb 27, 2023

The examples are failing
https://app.circleci.com/pipelines/github/pytorch/rl/6511/workflows/3deb03f3-aa94-400c-8756-895bfa050557/jobs/200111
Can you check why?

@BY571
Copy link
Contributor Author

BY571 commented Feb 28, 2023

The examples are failing https://app.circleci.com/pipelines/github/pytorch/rl/6511/workflows/3deb03f3-aa94-400c-8756-895bfa050557/jobs/200111 Can you check why?

It seems like the test can't handle the situation when wandb asks you to select wandb setup options:
image

I changed now the logging to tensorboard in the tests. This should fix it for now but not sure if it's a permanent solution.

@vmoens
Copy link
Contributor

vmoens commented Feb 28, 2023

The examples are failing https://app.circleci.com/pipelines/github/pytorch/rl/6511/workflows/3deb03f3-aa94-400c-8756-895bfa050557/jobs/200111 Can you check why?

It seems like the test can't handle the situation when wandb asks you to select wandb setup options: image

I changed now the logging to tensorboard in the tests. This should fix it for now but not sure if it's a permanent solution.

Can't you put it in local mode? I thing this can be done via an env variable.

Also there's an error remaining in TD3

TypeError: distribution keywords and tensordict keys indicated by ProbabilisticTensorDictModule.in_keys must match.Got this error message: 
    __init__() got an unexpected keyword argument 'tanh_loc'

@BY571
Copy link
Contributor Author

BY571 commented Mar 1, 2023

fixed the td3 issues and added the mode parameter so that for the tests wandb can be run in offline mode.

@vmoens
Copy link
Contributor

vmoens commented Mar 9, 2023

Sorry for dropping the ball
Can you merge main in this branch and let me know if I can help in any way?

@vmoens vmoens added the new algo New algorithm request or PR label Mar 13, 2023
vmoens
vmoens approved these changes Mar 14, 2023
View reviewed changes
Copy link
Contributor

@vmoens vmoens left a comment

Choose a reason for hiding this comment

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

LGTM

@vmoens vmoens merged commit 878d023 into pytorch:main Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vmoens vmoens vmoens approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. new algo New algorithm request or PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@BY571 @vmoens @facebook-github-bot