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

Feature/low rank approx #365

Merged
merged 49 commits into from
Jul 28, 2023
Merged

Feature/low rank approx #365

merged 49 commits into from
Jul 28, 2023

Conversation

schroedk
Copy link
Collaborator

@schroedk schroedk commented Jun 22, 2023

Description

This PR adds a new solver to compute influence functions. It is based on a low rank approximation, via approximate eigen decompositions. It calls ARPack routines via scipy, which seems to be good thing to do, in comparison to implement the Arnoldi/Lanczos in (cpu) torch.

Considerations to still mark this as draft:

  • I followed the existing abstraction (TorchTwiceDifferentiable), but would prefer to get rid of it, as it seems quite inflexible to me, also there is some broken signature, as the type hint suggest to follow the base class TwiceDifferentiable, while it actually needs TorchTwiceDifferentiable. I will create an issue for this. Should this be in this PR? I prefer to do it an a further PR separate PR
  • There is a possibility to do all computations on GPU (https://docs.cupy.dev/en/stable/reference/generated/cupyx.scipy.sparse.linalg.eigsh.html), actually possible with zero copy costs (https://docs.cupy.dev/en/stable/user_guide/interoperability.html#pytorch) I would prefer to have a new PR for this as well -> implmented in this PR
  • Once we streamlined the interfaces, I we could add support for JAX as well. This will be a large PR, I guess separate PR

This PR closes #282

Changes

  • add functionality to solve hessian equation via low rank approximation
  • add and refactor tests
  • fix type-check and linting issues

Checklist

  • Wrote Unit tests (if necessary)
  • Updated Documentation (if necessary)
  • Updated Changelog
  • If notebooks were added/changed, added boilerplate cells are tagged with "nbsphinx":"hidden"

@schroedk schroedk requested a review from mdbenito June 22, 2023 13:12
@mdbenito mdbenito mentioned this pull request Jun 22, 2023
4 tasks
@mdbenito mdbenito added the new-method Implementation of new algorithms for valuation or influence functions label Jun 22, 2023
@mdbenito mdbenito linked an issue Jun 22, 2023 that may be closed by this pull request
@schroedk schroedk changed the title Draft: Feature/low rank approx Feature/low rank approx Jun 26, 2023
Avoid confusion with TensorType defined in influence.frameworks.twice_differentiable, which serves a different purpose
* harmonize docstring format
* add unit test for align_structure
* move import to toplevel
* simplify path existence check
…d torch environment, in order to not invoke tests from other directories like /notebooks"

This reverts commit 5a28716.
@AnesBenmerzoug
Copy link
Collaborator

@schroedk There is a bug in Github with using [skip ci] as the last commit before a merge. It prevents CI from running and because CI needs to be successful in order to merge the PR that means we're stuck in a deadlock. We can either push a dummy commit (not really recommended) or just override the merge requirements (if you can't do it then just tell and I can do it for you).

@schroedk schroedk merged commit ecda48d into develop Jul 28, 2023
12 checks passed
@schroedk schroedk deleted the feature/low_rank_approx branch August 1, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-method Implementation of new algorithms for valuation or influence functions
Projects
None yet
4 participants