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

IF refactoring and further testing #278

Merged
merged 63 commits into from
May 19, 2023
Merged

Conversation

Xuzzo
Copy link
Collaborator

@Xuzzo Xuzzo commented Feb 24, 2023

Description

This PR closes #137 and closes #169 and closes #203 and closes #130 and closes #168 and closes #306
It also closes #222: wrappers have been removed and only one fitting method is used, now outside src (put in notebook_support).

Changes

  • Several refactorings of influence functions code
  • included if tests with several NN architectures and more detailed ones using linear models
  • removed useless framework wrappers and boilerplate from source code
  • Re-runs notebooks with new (slightly) modified interface

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"

@Xuzzo Xuzzo changed the title Test influence refactoring IF refactoring and further testing Feb 24, 2023
@Xuzzo Xuzzo changed the title IF refactoring and further testing Draft: IF refactoring and further testing Feb 24, 2023
Copy link
Collaborator

@mdbenito mdbenito left a comment

Choose a reason for hiding this comment

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

I've added a few comments in two files but I'm calling it a day. Will continue soon

src/pydvl/influence/frameworks/torch_differentiable.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support.py Outdated Show resolved Hide resolved
@mdbenito mdbenito marked this pull request as draft February 25, 2023 19:09
Copy link
Collaborator

@mdbenito mdbenito left a comment

Choose a reason for hiding this comment

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

Added a few more comments and some discussion on how to avoid the checks for modules inside the functions, which might pave the way towards #282

CHANGELOG.md Outdated Show resolved Hide resolved
tests/influence/conftest.py Outdated Show resolved Hide resolved
tests/influence/conftest.py Outdated Show resolved Hide resolved
tests/influence/conftest.py Outdated Show resolved Hide resolved
tests/influence/conftest.py Outdated Show resolved Hide resolved
src/pydvl/influence/frameworks/torch_differentiable.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mdbenito mdbenito left a comment

Choose a reason for hiding this comment

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

Added a few more comments

notebooks/notebook_support.py Outdated Show resolved Hide resolved
notebooks/notebook_support_torch.py Outdated Show resolved Hide resolved
src/pydvl/influence/frameworks/torch_differentiable.py Outdated Show resolved Hide resolved
src/pydvl/influence/general.py Outdated Show resolved Hide resolved
src/pydvl/influence/general.py Outdated Show resolved Hide resolved
src/pydvl/influence/inversion_methods.py Outdated Show resolved Hide resolved
src/pydvl/influence/inversion_methods.py Outdated Show resolved Hide resolved
src/pydvl/influence/types.py Outdated Show resolved Hide resolved
src/pydvl/influence/general.py Outdated Show resolved Hide resolved
src/pydvl/influence/general.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@mdbenito mdbenito left a comment

Choose a reason for hiding this comment

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

Lots of thing still to do, but we need to merge

@mdbenito mdbenito merged commit d5381d7 into develop May 19, 2023
@mdbenito mdbenito deleted the test_influence_refactoring branch May 19, 2023 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup when code is ugly or unreadable and needs restyling testing Writing and verifying tests (unit or otherwise)
Projects
None yet
3 participants