stable-baselines icon indicating copy to clipboard operation
stable-baselines copied to clipboard

Revision of CnnLstmPolicy with not None net_arch

Open HighExecutor opened this issue 4 years ago • 5 comments

#1117 Description When CnnLstmPolicy uses not None net_arch, cnn_extractor is used to perform features extraction inside LstmPolicy class. Here NotImplementedError is solved with usage of cnn_extractor. Test file is added.

Motivation and Context

  • [x] I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Documentation (update in the documentation)

Checklist:

  • [x] I've read the CONTRIBUTION guide (required)
  • [x] I have updated the changelog accordingly (required).
  • [ ] My change requires a change to the documentation.
  • [x] I have updated the tests accordingly (required for a bug fix or a new feature).
  • [ ] I have updated the documentation accordingly.
  • [x] I have ensured pytest and pytype both pass (by running make pytest and make type).

HighExecutor avatar May 22 '21 22:05 HighExecutor

Thanks for the PR! Please complete all the required steps in the PR template.

Miffyli avatar May 23 '21 10:05 Miffyli

I added the issue and edited PR.

HighExecutor avatar May 23 '21 12:05 HighExecutor

Hmm while I agree such functionality would be nice, it would differ from FeedForwardPolicy's behaviour where net_arch is ignored. I am not sure about changing functionality this late into the lifetime of the the package, or at very least both Lstm and FeedForward policies should have the same behaviour (in fact, FeedForward policy should probably throw an exception too on net_arch is not None).

@araffin comments? I favor not changing stuff, given maintenance mode of SB2.

Miffyli avatar May 26 '21 21:05 Miffyli

@araffin comments? I favor not changing stuff, given maintenance mode of SB2.

I would agree with this. We mostly accept only bug fixes and non-breaking changes now. And if you need it, you can define a custom policy.

araffin avatar May 27 '21 10:05 araffin

great. hope to see more of this

carroll1100 avatar Mar 27 '23 02:03 carroll1100