hill-a / stable-baselines

A fork of OpenAI Baselines, implementations of reinforcement learning algorithms
http://stable-baselines.readthedocs.io/
MIT License
4.14k stars 723 forks source link

Revision of CnnLstmPolicy with not None net_arch #1116

Open HighExecutor opened 3 years ago

HighExecutor commented 3 years ago

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

Types of changes

Checklist:

Miffyli commented 3 years ago

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

HighExecutor commented 3 years ago

I added the issue and edited PR.

Miffyli commented 3 years ago

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.

araffin commented 3 years ago

@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.

carroll1100 commented 1 year ago

great. hope to see more of this