-
Notifications
You must be signed in to change notification settings - Fork 9
Ekfac #13
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
Open
LouisYRYJ
wants to merge
106
commits into
main
Choose a base branch
from
ekfac
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will make future diffs easier to read. Done using: uv run --with jupyter jupyter nbconvert --ClearOutputPreprocessor.enabled=True --inplace tests/ekfac_tests/compute_ekfac_ground_truth.ipynb
This class was removed in 8232b77 but the notebook code was not adapted.
This cell contains code from tests/ekfac_tests/test_covariance.py but fails with file not found errors and isn't actually needed in the rest of the notebook.
This makes it easier to run the script on a small GPU for testing.
This is adapted from the main branch of bergson.
… a bit more specific
Use jupytext percent format which is directly interpretable by vscode (and can also be converted into ipynb using `jupytext --to notebook`). We want compute_ekfac_ground_truth to be: - Importable from other files. So it should be split in functions and importing it shouldn't have side-effects. - Usable a script we run from. So it should have a main that parses input arguments and run everything. - Usable as a notebook. So it should be split into cells where each cell can be executed individually and produce some output. To gain back usability as a notebook without compromising the other usecases, we split the logic that used to be in `main()` in multiple statements guarded by `if __name__ == "__main__"` at the end of the cell that defines the relevant function (since each of these guarded statement defines some variable, we actually need `or TYPE_CHECKING` to ensure they are visible to the typechecker).
All covariance-related cells are next to each other, same for all eigendecomposition-related code.
ekfac: fix compute_ekfac_ground_truth, add minimal CI
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This introduces EKFAC computations, everything relevant is in /hessians
->As I refactored some parts of the general pipeline, we should probably merge ultrafast first and then I can make the changes to this PR.