Skip to content

Added BOS equal shares and fractional equal shares with tests#69

Open
IvanGorbache wants to merge 14 commits into
COMSOC-Community:mainfrom
IvanGorbache:main
Open

Added BOS equal shares and fractional equal shares with tests#69
IvanGorbache wants to merge 14 commits into
COMSOC-Community:mainfrom
IvanGorbache:main

Conversation

@IvanGorbache

Copy link
Copy Markdown

Hello,
I have created a fork that adds two new methods for budgeting, BOS equal shares and fractional equal shares, as well as tests for the new methods. Their implementation is based on the pseudocode outlined in the attached article

bos_equal_shares.py
test_rule.py
50.pdf

Thank you in advance!

Comment thread pabutools/rules/bos_equal_shares.py Outdated

return dict(sorted(project_part.items(), key=lambda item: str(item[0])))


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.

Can you remove the main please. It's not needed in the package.

return voter.utility(project)
return 1 if project in voter else 0


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.

Can you please type all the functions as done in the other files of the package.

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.

This needs to be uniform with the rest of the package.

Comment thread pabutools/rules/mes/bos_equal_shares.py
Comment thread pabutools/rules/mes/bos_equal_shares.py
Comment thread tests/rules/test_bos.py

@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 your submission, I have added important comments to uniform your work with the rest of the package

@IvanGorbache

Copy link
Copy Markdown
Author

I will add the needed changes as soon as possible

- moved files to correct directories
- add checks for profile and ballot types
- change pytest to unitest
- added parameters and return to the function description
@IvanGorbache

Copy link
Copy Markdown
Author

I have made the needed changes

@IvanGorbache IvanGorbache requested a review from Simon-Rey June 10, 2026 06:11

@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.

Almost there, you still need to:

  1. Type the functions as done elsewhere in the package
  2. Update the docstring of the functions to match the package convention.

Comment thread pabutools/rules/mes/bos_equal_shares.py Outdated
"""
if not isinstance(profile, (ApprovalProfile, CardinalProfile)):
raise TypeError("profile must be an instance of ApprovalProfile or CardinalProfile")
if any(not isinstance(voter, (ApprovalBallot, CardinalBallot)) for voter in profile):

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.

This is not needed, it costs time to do that, let's just assume that if the right profile is passed, the ballot type is correct ;)

return voter.utility(project)
return 1 if project in voter else 0


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.

This needs to be uniform with the rest of the package.

@Simon-Rey

Copy link
Copy Markdown
Member

As an example, here is the typing and docstring for MES:

    instance: Instance,
    profile: AbstractProfile,
    sat_class: type[SatisfactionMeasure] | None = None,
    sat_profile: GroupSatisfactionMeasure | None = None,
    tie_breaking: TieBreakingRule | None = None,
    resoluteness: bool = True,
    initial_budget_allocation: Iterable[Project] | None = None,
    voter_budget_increment=None,
    binary_sat=None,
    skipped_project: Project | None = None,
    analytics: bool = False,
    verbose: bool = False,
) -> BudgetAllocation | list[BudgetAllocation]:
    """
    The Method of Equal Shares (MES). See the website `equalshares.net <https://equalshares.net/>`_
    for details about how to compute the outcome of the rule. Note that the satisfaction measure is
    assumed to be additive.

    Parameters
    ----------
        instance: :py:class:`~pabutools.election.instance.Instance`
            The instance.
        profile : :py:class:`~pabutools.election.profile.profile.AbstractProfile`
            The profile.
        sat_class : type[:py:class:`~pabutools.election.satisfaction.satisfactionmeasure.SatisfactionMeasure`]
            The class defining the satisfaction function used to measure the social welfare. It should be a class
            inheriting from pabutools.election.satisfaction.satisfactionmeasure.SatisfactionMeasure.
            If no satisfaction is provided, a satisfaction profile needs to be provided. If a satisfaction profile is
            provided, the satisfaction argument is disregarded.
        sat_profile : :py:class:`~pabutools.election.satisfaction.satisfactionmeasure.GroupSatisfactionMeasure`
            The satisfaction profile corresponding to the instance and the profile. If no satisfaction profile is
            provided, but a satisfaction function is, the former is computed from the latter.
        initial_budget_allocation : Iterable[:py:class:`~pabutools.election.instance.Project`]
            An initial budget allocation, typically empty.
        tie_breaking : :py:class:`~pabutools.tiebreaking.TieBreakingRule`, optional
            The tie-breaking rule used.
            Defaults to the lexicographic tie-breaking.
        resoluteness : bool, optional
            Set to `False` to obtain an irresolute outcome, where all tied budget allocations are returned.
            Defaults to True.
        voter_budget_increment : Numeric, optional
            Any value that is not `None` will lead to the iterated variant of MES where `voter_budget_increment` units
            of budget are added to the initial budget of the voters until an exhaustive budget allocation is found, or
            one that is no longer feasible with the initial budget constraint.
        binary_sat : bool, optional
            Uses the inner algorithm for binary satisfaction if set to `True`. Should typically be used with approval
            ballots to gain on the runtime. Automatically set to `True` if an approval profile is given.
        skipped_project: MESProject, optional,
            Project from instance which shouldn't be considered in calculations and for which effective support
            will be calculated, if analytics is true. Solely used by analytics module.
        analytics: bool, optional
            (De)Activate the computation of analytics. These are additional details that can be accessed from the
            :py:class:`~pabutools.rules.budgetallocation.BudgetAllocation` object returned by the rule to perform
            analyses.
            Defaults to `False`.
        verbose : bool, optional
            (De)Activate the display of additional information.
            Defaults to `False`.

    Returns
    -------
        :py:class:`~pabutools.rules.budgetallocation.BudgetAllocation` | list[:py:class:`~pabutools.rules.budgetallocation.BudgetAllocation`]
            The selected projects if resolute (:code:`resoluteness == True`), or the set of selected projects if irresolute
            (:code:`resoluteness == False`).
    """```

updated the functions and tests to be in line with the rest of the package
@IvanGorbache IvanGorbache requested a review from Simon-Rey June 11, 2026 10:11
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.

2 participants