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

[RLlib] MARWIL does is+wis OPE by default #26667

Closed
Rohan138 opened this issue Jul 18, 2022 · 1 comment · Fixed by #26893
Closed

[RLlib] MARWIL does is+wis OPE by default #26667

Rohan138 opened this issue Jul 18, 2022 · 1 comment · Fixed by #26893
Assignees
@Rohan138
Labels
bug Something that is supposed to be working; but isn't P2 Important issue, but not time-critical rllib RLlib related issues rllib-offline-rl Offline RL problems

Comments

@Rohan138
Copy link
Contributor

Rohan138 commented Jul 18, 2022 •
edited
Loading

What happened + What you expected to happen

MARWIL currently runs ImportanceSampling and WeightedImportanceSampling OPE by default.

This isn't really good design and has caused CI/release test problems-we should set it to None and have the user specify it if necessary.

We'll have to figure out how to deprecate this while warning users that may be using MARWIL with the current default.

Versions / Dependencies

ray: master

Reproduction script

https://github.com/ray-project/ray/blob/master/rllib/algorithms/marwil/marwil.py#L107

Issue Severity

Low: It annoys or frustrates me.
@Rohan138 Rohan138 added P2 Important issue, but not time-critical rllib RLlib related issues rllib-offline-rl Offline RL problems labels Jul 18, 2022
@Rohan138 Rohan138 self-assigned this Jul 18, 2022
@Rohan138 Rohan138 added the bug Something that is supposed to be working; but isn't label Jul 18, 2022
@Rohan138 Rohan138 changed the title RLlib: MARWIL does is+wis OPE by default [RLlib] MARWIL does is+wis OPE by default Jul 20, 2022
@Rohan138 Rohan138 mentioned this issue Jul 22, 2022
Merged
6 tasks
@Rohan138
Copy link
Contributor Author

Opened PR in #26893, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Rohan138 Rohan138

Labels
bug Something that is supposed to be working; but isn't P2 Important issue, but not time-critical rllib RLlib related issues rllib-offline-rl Offline RL problems
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[RLlib]: Raise deprecation warning in MARWIL OPE methods Rohan138/ray
1 participant
@Rohan138