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

Added cstv algorithm and tests. #29

Merged
merged 102 commits into from
Jul 3, 2024
Merged

Added cstv algorithm and tests. #29

merged 102 commits into from
Jul 3, 2024

Conversation

tjhv10
Copy link
Contributor

@tjhv10 tjhv10 commented Jun 23, 2024

An implementation of the algorithms in:
"Participatory Budgeting with Cumulative Votes", by Piotr Skowron, Arkadii Slinko, Stanisaw Szufa, Nimrod Talmon (2020), https://arxiv.org/pdf/2009.02690
Programmer: Achiya Ben Natan
Date: 2024/05/16.

@Simon-Rey
Copy link
Member

I'll have a new look soon.

About resoluteness: the idea is the following: it may be that the rule can at some point choose between several projects (for instance because they all have the same score). If resoluteness is set to False, in this case you need to output all the budget allocations that could be selected (based on any arbitrary choice of ties projects). To give you another example it would consist is returning all the maximum flows in a flow algorithm instead of just one maximum.

I'm surprised that you are saying that there is no tie breaking. For instance if all projects have the same cost and all voters give the same contribution to all projects, what happens?

@erelsgl
Copy link
Contributor

erelsgl commented Jun 28, 2024

@Simon-Rey Regarding irresolute rules: since the rule is sequential, I am not sure how to make them irresolute. Does it require to keep a tree with all possibilities in each step? This tree might grow very large and complex if there are many iterations.

@Simon-Rey
Copy link
Member

Yes it's bad in terms of complexity, but it's anyways a feature that is only interesting on small instances. You can check the implementation of the greedy max welfare, there is a all_budget_allocations list that is populated by side effect. (https://github.com/pbvoting/pabutools/blob/main/pabutools/rules/greedywelfare/greedywelfare_rule.py)

@tjhv10
Copy link
Contributor Author

tjhv10 commented Jul 2, 2024

Hi, I added tie breaking to the algorithm. I didn't succeed to add the resoluteness = False feature so I added this error: if not resoluteness:
raise NotImplementedError('The "resoluteness = False" feature is not yet implemented'). If you want to see the closest I got to add the resoluteness = False feature you can check it in the
best try for resulteness commit which is here: 21d3e64

@Simon-Rey
Copy link
Member

That looks all good to me. Thanks for the effort you put into this :)

@Simon-Rey Simon-Rey merged commit 419daac into pbvoting:main Jul 3, 2024
3 checks passed
@Simon-Rey
Copy link
Member

Ok I looked more carefuly at the code now and I have changed a lot of things. Many things were not correct, the typing notably but also the use of the different elements of the package. You can check my changes, let me know if you spot any mistake. I still need to update the tests.

@tjhv10
Copy link
Contributor Author

tjhv10 commented Jul 3, 2024

Ok I looked more carefuly at the code now and I have changed a lot of things. Many things were not correct, the typing notably but also the use of the different elements of the package. You can check my changes, let me know if you spot any mistake. I still need to update the tests.

I looked throw the code and you really changed a lot. I think the names of the functions you changed their name should remain the names I did because then it is easier to compare the code to the paper the code is based on but it is your library and you can do what ever you like. If you wander what is this line: "donor[chosen_project] += (np.ceil(change * 100000000000000)/ 100000000000000) # TODO: What is this?? " it is the easiest way I found to round up a number in the 14th number after the point which is a crucial feature that the code will not work without it for now because of rounding errors in python which makes the r to be 0.999999999999999999 instead of 1 and then the while loop will not stop running. If you find a better way to change the r to 1 after one iteration instead of the while running a few times to fix the rounding mistake of python please change this part, it will make the minimal transfer function to work a lot faster and better.

@Simon-Rey
Copy link
Member

Ok for the fraction, I anyways need to update the code so that it uses the function functionalities of the package (because we already have code to deal with fractions).

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.

3 participants