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] Simplify DirectionalDerivatives API #17

Merged
merged 15 commits into from Feb 21, 2022

Conversation

f-dangel
Copy link
Owner

@f-dangel f-dangel commented Feb 18, 2022

Exotic features, like using different GGNs to compute directions and
directional curvatures, as well as full control of which intermediate buffers
to keep, have been deprecated in favor of a simpler API.

  • Remove Newton step computation for now as it was internally relying on
    DirectionalDerivatives
  • Remove many utilities and associated tests from the exotic features
  • Forbid duplicate indices in subsampling
  • Always delete intermediate buffers other than the target quantities

f-dangel and others added 14 commits February 14, 2022 15:59
Always compute and keep first-order directional derivatives.

s
Currently, the Newton step depends on the directional derivatives.
I will be easier to clean up the latter's API before reactivating the
Newton step feature.
Always delete the Gram matrix.
Always remove the Gram matrix eigenvectors.
Always delete the Gram matrix eigenvalues.
Always delete batch size.
Always delete BackPACK buffers.
* [REF] Rename file containing argument checks

* [ADD] Forbid sub-sampling with repeated indices

Co-authored-by: Felix Dangel <fdangel@tue.mpg.de>
Forbid duplicates in subsampling, share subsampling for directions and
directional curvatures.
@f-dangel f-dangel changed the title [ADD] Simplify DirectionalDerivatives [ADD] Simplify DirectionalDerivatives API Feb 18, 2022
@coveralls
Copy link

coveralls commented Feb 18, 2022

Pull Request Test Coverage Report for Build 1864267792

  • 177 of 199 (88.94%) changed or added relevant lines in 3 files are covered.
  • 11 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-1.06%) to 80.722%

Changes Missing Coverage Covered Lines Changed/Added Lines %
vivit/optim/directional_derivatives.py 172 194 88.66%
Files with Coverage Reduction New Missed Lines %
vivit/utils/ggn.py 11 50.0%
Totals Coverage Status
Change from base Build 1864018123: -1.06%
Covered Lines: 850
Relevant Lines: 1053

💛 - Coveralls

@f-dangel f-dangel merged commit 5310b0c into development Feb 21, 2022
@f-dangel f-dangel deleted the directional-derivatives branch February 22, 2022 16:35
f-dangel added a commit that referenced this pull request Mar 24, 2022
Exotic features, like using different GGNs to compute directions and
directional curvatures, as well as full control of which intermediate buffers
to keep, have been deprecated in favor of a simpler API.

- Remove Newton step computation for now as it was internally relying on
 `DirectionalDerivatives`
- Remove many utilities and associated tests from the exotic features
- Forbid duplicate indices in `subsampling`
- Always delete intermediate buffers other than the target quantities

---

* [REF] Rename `GramComputations` → `DirectionalDerivativesComputation`

* [DEL] Remove `compute_gammas` and `keep_gammas` arguments

Always compute and keep first-order directional derivatives.

* [DEL] Remove `compute_lambdas` and `keep_lambdas` arguments

* [DEL] Remove Newton step and Newton optimizer

Currently, the Newton step depends on the directional derivatives.
I will be easier to clean up the latter's API before reactivating the
Newton step feature.

* [DEL] Remove `keep_gram_mat` argument

Always delete the Gram matrix.

* [DEL] Remove `keep_gram_evecs` argument

Always remove the Gram matrix eigenvectors.

* [DEL] Remove `keep_gram_evals` argument

Always delete the Gram matrix eigenvalues.

* [DEL] Remove `keep_batch_size` argument

Always delete batch size.

* [DEL] Remove `keep_backpack_buffers` argument

Always delete BackPACK buffers.

* [DEL] Remove `param_groups` from `get_extensions`

* [ADD] Simplify `DirectionalDerivativesComputation` API and tests

* [ADD] Forbid `subsampling` with repeated indices (#16)

* [REF] Rename file containing argument checks

* [ADD] Forbid sub-sampling with repeated indices

Co-authored-by: Felix Dangel <fdangel@tue.mpg.de>

* [ADD] Simplify internals of `DirectionalDerivativesComputation`

Forbid duplicates in subsampling, share subsampling for directions and
directional curvatures.

Co-authored-by: Felix Dangel <fdangel@tue.mpg.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants