Skip to content

Feature: Add Fair Lotteries algorithms and GCR baseline#67

Open
dotandanino wants to merge 39 commits into
COMSOC-Community:mainfrom
dotandanino:main
Open

Feature: Add Fair Lotteries algorithms and GCR baseline#67
dotandanino wants to merge 39 commits into
COMSOC-Community:mainfrom
dotandanino:main

Conversation

@dotandanino

Copy link
Copy Markdown

Description

This PR introduces the randomized algorithms from the AAAI-24 paper "Fair Lotteries for Participatory Budgeting," alongside a baseline implementation of the Greedy Cohesive Rule (GCR).

Files Added

  • Fair_Lotteries_for_Participatory_Budgeting.py
  • pabutools/rules/gcr/ (module files)
  • pabutools/rules/lottery/ (module files)
  • tests/test_bw_algorithms.py

Testing

  • All 151 tests and 85 subtests pass successfully locally.

Comment thread pabutools/rules/gcr/gcr_rule.py
Comment thread pabutools/rules/lottery/lottery_rule.py Outdated
Comment thread pabutools/rules/lottery/lottery_rule.py
Comment thread pabutools/rules/lottery/lottery_rule.py
Comment thread Fair_Lotteries_for_Participatory_Budgeting.py Outdated
Comment thread pabutools/analysis/justifiedrepresentation.py Outdated
Comment thread pabutools/analysis/justifiedrepresentation.py Outdated
Comment thread pabutools/analysis/justifiedrepresentation.py Outdated

@Simon-Rey Simon-Rey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you very much for your submission. I have quickly looked at the formatting/writing up of the code. There are few comments of the things I would like you to fix before merging ;)

I have not checked the code itself, I assume that you have included sufficient tests to trust that the functions behave as expected.

Comment thread .gitignore
Comment thread .flake8 Outdated
Comment thread pabutools/fractions.py Outdated
Comment thread pabutools/election/profile/approvalprofile.py
Comment thread pabutools/analysis/justifiedrepresentation.py Outdated

@Simon-Rey Simon-Rey left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the improvements. I've made some new comments, we're almost there ;)

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 79.09605% with 74 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.81%. Comparing base (49105f3) to head (36eccd7).
⚠️ Report is 35 commits behind head on main.

Files with missing lines Patch % Lines
pabutools/analysis/justifiedrepresentation.py 15.21% 39 Missing ⚠️
pabutools/rules/lottery/lottery_rule.py 84.21% 33 Missing ⚠️
pabutools/rules/gcr/gcr_rule.py 96.61% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #67      +/-   ##
==========================================
- Coverage   85.33%   84.81%   -0.52%     
==========================================
  Files          53       58       +5     
  Lines        4165     4518     +353     
==========================================
+ Hits         3554     3832     +278     
- Misses        611      686      +75     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

These are course material and a separate website repo, not part of
the pabutools library. Both are now covered by .gitignore.

@dotandanino dotandanino left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hello dear Simon, We fixed the comments and would like recheck

@Simon-Rey

Copy link
Copy Markdown
Member

First the tests fail, so please fix and re-run the tests before pushing.

@Simon-Rey

Copy link
Copy Markdown
Member

Then, I don't really understand the functions in "justifiedrepresentation.py". I am not sure they deserve to be in the package. One reason is that they all use objects and types that are different from the ones used in the package (cost dicts instead of simply the in-built Project class). Second reason is that I am not fully convinced by the "random" function that just sample some groups. I don't really see why would people use that. If anything, I would want a full perfect check that maybe has an extra parameter to just sample few groups. Do you need these functions elsewhere? If not, then I would just not include them in the PR.

dotandanino and others added 2 commits June 30, 2026 15:50
A prior merge on main left the lottery import block unclosed while
splicing in ordered_relax/ees_addopt imports, causing a SyntaxError
that failed flake8/CI on every Python version. Properly close both
import blocks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace check_FJR/check_EJR/check_strong_UFS, which operated on raw
dicts, with is_FJR_approval/is_EJR_approval (now backed by
cohesive_groups)/is_strong_UFS_approval, all using Instance,
AbstractApprovalProfile, Project and SatisfactionMeasure directly.
check_EJR was dropped as a duplicate of the existing is_EJR_approval.

cohesive_groups and the new checks gain an optional sample_size
parameter: omitted, they run exhaustively (suitable for small test
instances); set, they bound the search to randomly sampled groups so
checks stay tractable on large instances.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

4 participants