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.

Sign up for GitHub

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

Jump to bottom

Implement class wise shapley #338

Merged
merged 53 commits into from
Oct 14, 2023
Merged

Implement class wise shapley #338

merged 53 commits into from
Oct 14, 2023

Conversation

kosmitive
Copy link
Contributor

@kosmitive kosmitive commented Apr 5, 2023 •
edited
Loading

Description

This PR closes #259

Changes

  • Added class wise shapley algorithm
  • Added test case for in and out of class accuracy.

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"

@kosmitive kosmitive added enhancement New feature or request new-method Implementation of new algorithms for valuation or influence functions paper reproduction Steps required to fully reproduce a paper labels Apr 5, 2023
@kosmitive kosmitive self-assigned this Apr 5, 2023
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch from 3fc3e5f to 0c51d75 Compare April 12, 2023 12:11
mdbenito
mdbenito reviewed Apr 15, 2023
View reviewed changes
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 added a few superficial comments but didn't review in depth. Waiting until you are done. Nice progress!

src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/result.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/utils/numeric.py Outdated Show resolved Hide resolved
src/pydvl/utils/utility.py Outdated Show resolved Hide resolved
mdbenito
mdbenito reviewed Apr 19, 2023
View reviewed changes
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, mostly about conventions. Please also respect the column limit in the docstrings (we use black's 88 column default)

tests/value/shapley/test_classwise.py Outdated Show resolved Hide resolved
tests/value/shapley/test_classwise.py Outdated Show resolved Hide resolved
tests/value/shapley/test_classwise.py Outdated Show resolved Hide resolved
tests/value/shapley/test_classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch 5 times, most recently from 79faa0b to 75c4aab Compare May 2, 2023 18:56
mdbenito
mdbenito reviewed May 7, 2023
View reviewed changes
Copy link
Collaborator

@mdbenito mdbenito left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I left more comments. There is so much "peripheral" stuff that I find it hard to focus on the core functionality, sorry. Please review everything for spelling, correctness and consistency before requesting another review. I strongly recommend using GPT-4 for this! Still, good progress 👍🏽

src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/utils/score.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch from 5ae205a to 497ee19 Compare July 25, 2023 04:43
@kosmitive kosmitive mentioned this pull request Jul 27, 2023
Closed
@AnesBenmerzoug AnesBenmerzoug mentioned this pull request Jul 27, 2023
Merged
4 tasks
@kosmitive kosmitive changed the title Draft: Implement class wise shapley Implement class wise shapley Jul 28, 2023
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch 4 times, most recently from b81f85e to 9301d4e Compare August 7, 2023 18:52
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch 4 times, most recently from 9862f18 to 4bd92ec Compare August 12, 2023 22:41
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch from 4bd92ec to 3e69df0 Compare August 14, 2023 19:48
mdbenito
mdbenito reviewed Aug 26, 2023
View reviewed changes
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.

Thanks for all the work. Now that you seem to have ironed out most bugs, I looked at this again. I left a few general comments, but haven't checked the algorithm itself. Let me know when these are addressed.

src/pydvl/utils/dataset.py Outdated Show resolved Hide resolved
src/pydvl/utils/score.py Outdated Show resolved Hide resolved
src/pydvl/value/stopping.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/truncated.py Outdated Show resolved Hide resolved
tests/misc.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/classwise.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
src/pydvl/value/shapley/montecarlo.py Outdated Show resolved Hide resolved
src/pydvl/utils/config.py Outdated Show resolved Hide resolved
kosmitive pushed a commit that referenced this pull request Aug 28, 2023
Remove setter method from dataset to fix #338 (comment)
4ca022c
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch from 512a256 to 037fa66 Compare September 23, 2023 00:50
@kosmitive kosmitive force-pushed the 259-implement-class-wise-shapley branch from 0066343 to cd01657 Compare October 6, 2023 12:43
mdbenito
mdbenito requested changes Oct 8, 2023
View reviewed changes
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 left a couple of comments but this is almost ready. I will wait until you make the changes then do a final review. Please check that my change to the ClasswiseScorer is correct (how did you test with f1 if the class didn't allow changing the metric?)

docs/value/classwise-shapley.md Outdated Show resolved Hide resolved
docs/value/classwise-shapley.md Outdated Show resolved Hide resolved
docs/value/classwise-shapley.md Show resolved Hide resolved
docs/value/img/classwise-shapley-discounted-utility-function.svg Outdated Show resolved Hide resolved
@mdbenito mdbenito assigned mdbenito and unassigned kosmitive Oct 8, 2023
@mdbenito
Copy link
Collaborator

Merging despite many issues that we must leave for a later release. See DM

@mdbenito mdbenito merged commit 1949fcc into develop Oct 14, 2023
@mdbenito mdbenito deleted the 259-implement-class-wise-shapley branch October 14, 2023 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AnesBenmerzoug AnesBenmerzoug AnesBenmerzoug left review comments

@mdbenito mdbenito mdbenito requested changes

Assignees

@mdbenito mdbenito

Labels
enhancement New feature or request new-method Implementation of new algorithms for valuation or influence functions paper reproduction Steps required to fully reproduce a paper
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement Class-Wise shapley
3 participants
@kosmitive @mdbenito @AnesBenmerzoug