-
Notifications
You must be signed in to change notification settings - Fork 8
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
Refactoring/influence #394
Conversation
* simplify grad method, do not return tuple, simplify input signature * simplify split_grad to call self.grad * change default argument of self.grad * extract split_grad method to simplify class
…s, add torch implementation
* introduce a dynamic registry for inversion methods * make top level modules independent of the torch implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice new design! Thanks a lot. I just have one comment on the concatenation of grads.
…igned with model named parameters), improve documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a somewhat superficial review since we already discussed most design choices. Left some ball-busting nit-picking comments, but I think this is great step forward.
* improve format * improve naming
* add missing methods to interface, remove obsolete ones, make cat preallocate tensor * add DataLoaderUtilities concept
* remove DataLoaderUtilities * try to call len on dataloader for pre-allocating result tensor, fallback to ordinary cat
Description
This PR introduces a cleanup on how we can handle different frameworks. I would like to start from this, to add the jax framework and an additional method (Nyström approximation #343). This is not breaking the user interface again, as it is only modifying internal mechanisms of the package.
Changes
Checklist
[ ] If notebooks were added/changed, added boilerplate cells are tagged with"nbsphinx":"hidden"