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.

Sign up for GitHub

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

Jump to bottom

Re-examine the whole state story for RNNs #1095

Open
MischaPanch opened this issue Apr 3, 2024 · 0 comments
Open

Re-examine the whole state story for RNNs #1095

MischaPanch opened this issue Apr 3, 2024 · 0 comments
Labels
refactoring No change to functionality RNN Temporary label to group all things RNN tentative Up to discussion, may be dismissed

Comments

@MischaPanch
Copy link
Collaborator

The state: Any | None = None crept into all forwards of children of NetBase. Properly accommodating it forced me to make NetBase generic, just to allow to do class Recurrent(NetBase[RecurrentStateBatch]).

The state is unused almost everywhere and it makes every signature more complicated. On top of that, RNN things are currently broken anyway. Maybe there's a way to make our lives simpler without completely throwing over board the possibility to repair RNNs down the line?

Also pinging @opcode81 @Trinkle23897 , do you have an opinion?
@MischaPanch MischaPanch added refactoring No change to functionality tentative Up to discussion, may be dismissed RNN Temporary label to group all things RNN labels Apr 3, 2024
@MischaPanch MischaPanch mentioned this issue Apr 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
refactoring No change to functionality RNN Temporary label to group all things RNN tentative Up to discussion, may be dismissed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@MischaPanch