HER Wrappers

[hades-rp2010]

Wrt #171
Have added a HERTrainer, HERGoalEnvWrapper, and a HERWrapper for the replay buffer.
Some changes in the locations of the tests might be needed.. Wasnt too sure of where to put them

[lgtm-com]

This pull request introduces 7 alerts when merging acd87f6 into 9b7400e - view on LGTM.com

new alerts:

• 7 for Unused import

[codecov]

Codecov Report

Merging #340 into master will decrease coverage by 0.76%.
The diff coverage is 78.59%.

@@            Coverage Diff             @@
##           master     #340      +/-   ##
==========================================
- Coverage   91.22%   90.45%   -0.77%     
==========================================
  Files          89       93       +4     
  Lines        3772     4014     +242     
==========================================
+ Hits         3441     3631     +190     
- Misses        331      383      +52     

Impacted Files	Coverage Δ
genrl/core/buffers.py	73.50% <47.27%> (-21.74%)	⬇️
genrl/environments/her_wrapper.py	82.14% <82.14%> (ø)
genrl/environments/custom_envs/BitFlipEnv.py	83.07% <83.07%> (ø)
genrl/utils/utils.py	94.31% <90.00%> (+0.06%)	⬆️
genrl/trainers/her_trainer.py	92.15% <92.15%> (ø)
genrl/agents/deep/base/offpolicy.py	97.40% <100.00%> (ø)
genrl/agents/deep/dqn/base.py	94.68% <100.00%> (+0.42%)	⬆️
genrl/core/__init__.py	100.00% <100.00%> (ø)
genrl/core/actor_critic.py	98.00% <100.00%> (+0.05%)	⬆️
genrl/environments/__init__.py	100.00% <100.00%> (ø)
... and 7 more

[lgtm-com]

This pull request introduces 2 alerts when merging 06cb5a7 into 9b7400e - view on LGTM.com

new alerts:

• 2 for Unused import

[Sharad24]

Awesome work! @hades-rp2010

Few questions:

1. Have you tried training?
2. Does the wrapper work with all off policy algorithms?
3. Have you checked out https://github.com/eleurent/highway-env? Highway is a standard goal based env.

[Sharad24]

Might need to refactor this -- arent there functions here which are exact duplicates of the ones in OffPolicyTrainer?

[Sharad24]

Should this be in the tests?

[Sharad24]

Maybe we could move this somewhere in the environments module.

[hades-rp2010]

Awesome work! @hades-rp2010

Few questions:

1. Have you tried training?
2. Does the wrapper work with all off policy algorithms?
3. Have you checked out https://github.com/eleurent/highway-env? Highway is a standard goal based env.

1. Still working on this, should mostly be over soon.
2. It works on all OffPolicy Agents, tested it on highway_env (Parking-v0) for cont. and BitFlipEnv for discrete
3. Yeah, I was trying to add tests for highway_env, but was running into some problems about installing the module in the lint tests. Would probs need some help on how to do this

[lgtm-com]

This pull request introduces 5 alerts when merging b654d8c into 147d373 - view on LGTM.com

new alerts:

• 5 for Unused import

[Sharad24]

Add highway_env to the pip install ... statements in .github/workflows/tests.yml and .github/workflows/codecov.yml, should work post that.

[lgtm-com]

This pull request introduces 5 alerts when merging da535e2 into 147d373 - view on LGTM.com

new alerts:

• 5 for Unused import

[lgtm-com]

This pull request introduces 5 alerts when merging 9edae64 into 147d373 - view on LGTM.com

new alerts:

• 5 for Unused import

[sampreet-arthi]

Are you done here? @hades-rp2010 If you can resolve the merge conflicts and maybe the codeclimate issues then we can merge this.

[Sharad24]

I think the algorithms weren’t training here. Please ensure you get a reasonable reward :)

…

On 12-Oct-2020, at 12:17 PM, Sampreet ***@***.***> wrote: Are you done here? @hades-rp2010 <https://github.com/hades-rp2010> If you can resolve the merge conflicts and maybe the codeclimate issues then we can merge this. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#340 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AH72FJYHZVDUIXFRDDDOUELSKKQ7TANCNFSM4RJ6TMOA>.