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

[ADD] Support for empirical Fisher in KFAC #54

Merged
merged 4 commits into from
Nov 7, 2023
Merged

[ADD] Support for empirical Fisher in KFAC #54

merged 4 commits into from
Nov 7, 2023

Conversation

runame
Copy link
Collaborator

@runame runame commented Oct 31, 2023

Resolves #46.

I have already added a 'type-2' option for the fisher_type argument. Not sure if I like the name since this terminology is not very common, but using 'exact' instead lead to confusion before.

@runame runame added the enhancement New feature or request label Oct 31, 2023
@runame runame requested a review from f-dangel October 31, 2023 20:23
@coveralls
Copy link

coveralls commented Oct 31, 2023

Pull Request Test Coverage Report for Build 6787848309

  • 11 of 13 (84.62%) changed or added relevant lines in 1 file are covered.
  • 40 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-5.2%) to 87.25%

Changes Missing Coverage Covered Lines Changed/Added Lines %
curvlinops/kfac.py 11 13 84.62%
Files with Coverage Reduction New Missed Lines %
curvlinops/examples/utils.py 6 41.67%
curvlinops/fisher.py 34 28.85%
Totals Coverage Status
Change from base Build 6697342688: -5.2%
Covered Lines: 698
Relevant Lines: 800

💛 - Coveralls

Copy link
Owner

@f-dangel f-dangel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me modulo the comments on documentation. Remind me whether there is already an issue to implement Fisher type-2 (edit: yes there is, #49).

curvlinops/kfac.py Outdated Show resolved Hide resolved
curvlinops/kfac.py Outdated Show resolved Hide resolved
curvlinops/kfac.py Show resolved Hide resolved
curvlinops/kfac.py Outdated Show resolved Hide resolved
@runame runame merged commit 238a7e4 into development Nov 7, 2023
13 checks passed
@runame runame deleted the kfac-ef branch November 7, 2023 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants