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

Remove kwargs in learn and update #949

Open
MischaPanch opened this issue Sep 24, 2023 · 1 comment
Open

Remove kwargs in learn and update #949

MischaPanch opened this issue Sep 24, 2023 · 1 comment
Labels
breaking changes Changes in public interfaces. Includes small changes or changes in keys major Large changes that cannot or should not be broken down into smaller ones refactoring No change to functionality
Milestone

Comments

@MischaPanch
Copy link
Collaborator

That's a tricky one since learn implementation are currently not cleanly separated (violate Liskov substitution) and rely on kwargs in order to remain functional. E.g. some implementations of learn take the repeat kwarg (the on-policy ones), and some don't.

Generally, the control flow between trainer.update_policy and policy.learn may need to be redesigned or at least improved

@MischaPanch MischaPanch added the refactoring No change to functionality label Sep 24, 2023
@MischaPanch MischaPanch added this to the Release 1.0.0 milestone Sep 24, 2023
@MischaPanch MischaPanch added the breaking changes Changes in public interfaces. Includes small changes or changes in keys label Nov 10, 2023
@MischaPanch
Copy link
Collaborator Author

@maxhuettenrauch @opcode81

This is the issue we talked about.

Solving it will likely be done by introducing a new Algorithm abstraction that incorporates some functionality of BasePolicy and some of BaseTrainer. This is a large change, but it's probably the only way to adequately address this issue.

Feel free to extend the issue description with more details of the current plan for solving this.

@MischaPanch MischaPanch added the major Large changes that cannot or should not be broken down into smaller ones label Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes Changes in public interfaces. Includes small changes or changes in keys major Large changes that cannot or should not be broken down into smaller ones refactoring No change to functionality
Development

No branches or pull requests

1 participant