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

[WIP] Added BCQ #378

Draft
wants to merge 20 commits into
base: master
Choose a base branch
from
Draft

Conversation

sampreet-arthi
Copy link
Member

@sampreet-arthi sampreet-arthi commented Oct 9, 2020

Stuff implemented:

  • Added BCQ under genrl/agents/offline
  • BCQ inherits from OffPolicyAgentAC. Architecture was very similar to TD3. Major differences were that the actor took in both state and action as input and the VAE obviously.
  • OfflineTrainer inherits from OffPolicyTrainer. Only difference is that it loads the buffer.
  • Refactored buffers and rollouts to inherit from BaseBuffer and remove redundant functions and converted all code to torch. No numpy is used in any of the buffer files now.

Stuff to do:

  • Haven't tested properly yet if it works. Currently created a toy replay buffer from DDPG on Pendulum-v0 with only 100 experiences
  • Will have to find a simple way to make the actor take in both state and action.

@sampreet-arthi
Copy link
Member Author

Buffers have been tested but not after the addition of BCQ so tests are failing rn

@lgtm-com
Copy link

lgtm-com bot commented Oct 9, 2020

This pull request introduces 3 alerts when merging 6c271ef into b8a45ab - view on LGTM.com

new alerts:

  • 1 for Unused local variable
  • 1 for Unused import
  • 1 for Wrong number of arguments in a call

@sampreet-arthi sampreet-arthi added this to In progress in Offline RL Oct 10, 2020
@codecov
Copy link

codecov bot commented Oct 10, 2020

Codecov Report

Merging #378 into master will decrease coverage by 2.76%.
The diff coverage is 58.76%.

@@            Coverage Diff             @@
##           master     #378      +/-   ##
==========================================
- Coverage   91.28%   88.51%   -2.77%     
==========================================
  Files          90       93       +3     
  Lines        3809     3944     +135     
==========================================
+ Hits         3477     3491      +14     
- Misses        332      453     +121     
Impacted Files Coverage Δ
genrl/agents/deep/base/base.py 93.75% <ø> (ø)
genrl/agents/deep/base/onpolicy.py 96.15% <ø> (ø)
genrl/trainers/onpolicy.py 92.00% <ø> (ø)
genrl/agents/offline/bcq/bcq.py 23.86% <23.86%> (ø)
genrl/trainers/offline.py 27.77% <27.77%> (ø)
genrl/core/models.py 33.33% <33.33%> (ø)
genrl/trainers/base.py 81.30% <47.05%> (-6.87%) ⬇️
genrl/core/buffers.py 92.94% <91.80%> (-2.30%) ⬇️
genrl/core/rollouts.py 96.77% <96.77%> (ø)
genrl/agents/__init__.py 100.00% <100.00%> (ø)
... and 13 more

@lgtm-com
Copy link

lgtm-com bot commented Oct 10, 2020

This pull request introduces 4 alerts when merging b28c1e6 into a2c8c7e - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 1 for Signature mismatch in overriding method

@lgtm-com
Copy link

lgtm-com bot commented Oct 16, 2020

This pull request introduces 4 alerts when merging 3db4733 into 25eb018 - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 1 for Signature mismatch in overriding method

@lgtm-com
Copy link

lgtm-com bot commented Oct 16, 2020

This pull request introduces 4 alerts when merging 43a483e into 25eb018 - view on LGTM.com

new alerts:

  • 3 for Unused import
  • 1 for Signature mismatch in overriding method

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Offline RL
In progress
Development

Successfully merging this pull request may close these issues.

None yet

1 participant