Title: Implemented GPseq algorithm #44

shlomiasraf · 2025-06-11T09:11:41Z

Full implementation of the Greedy Phragmén algorithm (GPseq) with doctests and logging.

Simon-Rey · 2025-06-11T12:27:39Z

The test should be implemented with unittest2. Take inspiration from the other tests in the package.

Simon-Rey · 2025-06-11T12:38:22Z

pabutools/rules/gpseq.py

+"""
+
+from __future__ import annotations
+from typing import List


Use the builtin type list and not this. The package typing should not be used now.

I changed it

Simon-Rey · 2025-06-11T12:38:35Z

pabutools/rules/gpseq.py

+from pabutools.election.ballot.approvalballot import ApprovalBallot
+from pabutools.tiebreaking import TieBreakingRule, lexico_tie_breaking
+import numpy as np
+import logging


No logging.

I changed it

Simon-Rey · 2025-06-11T12:38:54Z

pabutools/rules/gpseq.py

+    logging.info(f"project: {project.name} with cost {project.cost} and {float(max(new_loads))} max load")
+    return float(max(new_loads))
+
+if __name__ == '__main__':


Remove the main

I changed it.

Simon-Rey · 2025-06-11T12:39:18Z

tests/test_gpseq.py

@@ -0,0 +1,83 @@
+"""


This should be migrated to unittest2. The package does not use pytest.

I changed it.

Simon-Rey · 2025-06-11T12:39:59Z

See the comments I have left in the code and also for the other PR: (#43).

Simon-Rey · 2025-06-12T16:40:54Z

Looks good, two more things: first please remove all the DS_Store files, second remove the setup.py. We use the pyproject.toml to handle all the details of the package itself, there is no need for a setup.py.

shlomiasraf · 2025-06-13T09:28:23Z

Looks good, two more things: first please remove all the DS_Store files, second remove the setup.py. We use the pyproject.toml to handle all the details of the package itself, there is no need for a setup.py.

Ok, I deleted it.

Simon-Rey · 2025-06-14T13:19:24Z

Nice! Few extra things, sorry I had not paid attention to that before:

For fraction please don't use "/" but always frac from pabutools.fractions. That ensures proper support for the fractions functionnalities.
The rule should return a BudgetAlloction instance, not a list of Project.
Don't use len(profile) but profile.num_ballots() because if it's a MultiProfile then the two are different.
If the implementation does not work for MultiProfile, please check for that at the beginning and raise an error.
If the implementation also works for MultiProfile you probably want to make use of profile.multiplicity(ballot) to get the multiplicity of each ballot.
The profile type should be AbstractApprovalProfile

shlomiasraf · 2025-06-15T15:30:40Z

I added everything, please lets do it quickly.

Simon-Rey · 2025-06-15T16:07:44Z

Your code has indentation problems.

codecov-commenter · 2025-06-15T16:46:50Z

Codecov Report

Attention: Patch coverage is 92.06349% with 5 lines in your changes missing coverage. Please review.

Project coverage is 85.24%. Comparing base (1f89f0e) to head (7a4727a).
Report is 29 commits behind head on main.

Files with missing lines	Patch %	Lines
pabutools/rules/gpseq.py	92.06%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #44      +/-   ##
==========================================
- Coverage   89.67%   85.24%   -4.43%     
==========================================
  Files          49       50       +1     
  Lines        3641     3708      +67     
==========================================
- Hits         3265     3161     -104     
- Misses        376      547     +171

☔ View full report in Codecov by Sentry.
📢 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.

shlomiasraf · 2025-06-15T16:49:07Z

Fixed it, check now.

Simon-Rey · 2025-06-15T17:07:21Z

Sorry, I'm now confused what rule is it that you implemented? The article you refer to does not exist. I'm confused as to what the difference is compared to the Phragmén's rule already implemented. Maybe @erelsgl knows?

erelsgl · 2025-06-15T20:28:40Z

@Simon-Rey it is Algorithm 1 in this paper: https://arxiv.org/abs/1711.08226
@shlomiasraf there is a mistake in the author names - please fix

Simon-Rey · 2025-06-16T06:24:54Z

So for now the implementation is not that of Algo 1 from Aziz et al. (which, by the way, is the maximin support rule despite the name they gave it, see the survey). The current implementation does:

Choose greedily the projects one after the other
Select the project that minimises the maximum load of its approvers
The load is defined iteratively, starting from 0, each time a project is approved its supporters increase their load by c / num_supporters.

This is not the maximin support rule because in the latter the load is constantly recompute, i.e., the LP in Algo 1 is run at each iteration so the spread of the load of a project (uniform in the current implementation) can change at each iteration.

This is also not Sequential Phragmén as for the latter when selecting a project, the total load of its approvers is redistributed among them so that they all end up with the same load. Note that I have already implemented Sequential Phragmén in the package.

The implementation is a simplified version of sequential phragmen in which the load of a voter is monotonic. I am not aware of this rule having been studied and I'm now not super sure of the usefulness of having it in the package.

There may be something I missed in the implementation, please correct me if I'm wrong.

shlomiasraf · 2025-06-16T10:53:52Z

You're right, I've now updated the implementation so that the load is distributed optimally rather than uniformly, and it now matches the algorithm described in the paper.

Simon-Rey · 2025-06-16T10:55:46Z

Since the PR has been merged already you need to open a new one for me to see your code. Note that I have cleaned things around after the merge and that your code is now in the phragmen.py file.

shlomiasraf · 2025-06-16T10:59:20Z

Okay, I’ve opened another one.

shlomiasraf added 11 commits May 13, 2025 20:34

commit

32ec604

Update test_gpseq.py

22d4bd1

Update test_gpseq.py

71e84fa

Update test_gpseq.py

d36ae10

Update test_gpseq.py

ae98ecf

Update gpseq.py

3d662a1

Update test_gpseq.py

7b7ee88

Update gpseq.py

bec0f1f

Delete pabutools/gpseq.py

26cf15b

Add files via upload

5a86664

Update test_gpseq.py

feb7976

Simon-Rey reviewed Jun 11, 2025

View reviewed changes

shlomiasraf added 4 commits June 11, 2025 18:58

Update gpseq.py

20b4f2d

Update test_gpseq.py

df49b1e

Create setup.py

57b4efa

Update gpseq.py

03edbe9

shlomiasraf added 3 commits June 13, 2025 12:26

Delete pabutools/.DS_Store

a671144

Delete .DS_Store

e9a4350

Delete setup.py

825cbac

Update gpseq.py

d685b9d

Update gpseq.py

7a4727a

Simon-Rey merged commit 98d8bdd into COMSOC-Community:main Jun 15, 2025
5 checks passed

Simon-Rey mentioned this pull request Jun 16, 2025

Revert "Title: Implemented GPseq algorithm" #45

Closed

Title: Implemented GPseq algorithm #44

Title: Implemented GPseq algorithm #44

Uh oh!

Conversation

shlomiasraf commented Jun 11, 2025

Uh oh!

Simon-Rey commented Jun 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Simon-Rey commented Jun 11, 2025

Uh oh!

Simon-Rey commented Jun 12, 2025

Uh oh!

shlomiasraf commented Jun 13, 2025

Uh oh!

Simon-Rey commented Jun 14, 2025

Uh oh!

shlomiasraf commented Jun 15, 2025

Uh oh!

Simon-Rey commented Jun 15, 2025

Uh oh!

codecov-commenter commented Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

shlomiasraf commented Jun 15, 2025

Uh oh!

Simon-Rey commented Jun 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erelsgl commented Jun 15, 2025

Uh oh!

Simon-Rey commented Jun 16, 2025

Uh oh!

shlomiasraf commented Jun 16, 2025

Uh oh!

Simon-Rey commented Jun 16, 2025

Uh oh!

shlomiasraf commented Jun 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jun 15, 2025 •

edited

Loading

Simon-Rey commented Jun 15, 2025 •

edited

Loading