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

StableRoommates returning None for simple problems #124

Closed
MartinMohlenkamp opened this issue Oct 12, 2020 · 4 comments
Closed

StableRoommates returning None for simple problems #124

MartinMohlenkamp opened this issue Oct 12, 2020 · 4 comments

Comments

@MartinMohlenkamp
Copy link

This simple example:

import matching
from matching.games import StableRoommates
preferences = {'A' : ['B'], 'B' : ['A']}
game = StableRoommates.create_from_dictionary(preferences)
game.solve()

returns
{A: None, B: None}

I think the problem is in stable_roommates.py within forget_successors. Line 113 player.unmatch() erases the correct matching, which is then never recovered.

I have version '1.3.2'.

@daffidwilde
Copy link
Owner

Hi @MartinMohlenkamp, thanks for bringing this up but this behaviour is expected currently. The StableRoommates class implements Irving's algorithm which consists of two phases:

  • The first phase eliminates any unreasonable pairings (in this example there are none) through one-way, temporary proposals. The player.unmatch() line is required to clear up these proposals in any other example.
  • The second phase locates and removes all-or-nothing cycles from the preference lists (see the linked paper for details on that.) However, this stage requires at least one player to have more than one player in their preference list. As this is not the case here, the second_phase() function is never called and no matches are set.

I hope that clears things up. I appreciate that this appears rather unsatisfying, but I would prefer not to muck about with the algorithm implementation for the sake of a trivial case. Having said that, I'd happily accept a small catch in the StableRoommates.solve() method for when len(players) == 2.

Let me know your thoughts

@MartinMohlenkamp
Copy link
Author

@daffidwilde, the problem is not specific to two players. These also fail:

preferences = {'A' : ['B','C','D'],
          'B' : ['A','C','D'],
          'C' : ['A','B','D'],
          'D' : ['A','B','C']}

preferences = {'A' : ['B','C','D'],
          'B' : ['A','C','D'],
          'C' : ['D','B','A'],
          'D' : ['C','B','A']}

Based on your description, in any case when the second phase does not trigger, the code will fail. What if player.unmatch() was moved from the end of phase 1 to the beginning of phase 2? That would leave the matches intact when phase 2 is not called.

@daffidwilde
Copy link
Owner

Yes, you are absolutely right. That is quite the oversight by me.

Your solution sounds good, and I've run it through some other examples as well without issue. So, yeah, I'm happy for the unmatching to happen in the second phase 😄

daffidwilde added a commit that referenced this issue Oct 15, 2020
daffidwilde added a commit that referenced this issue Oct 15, 2020
* Move unmatching inside `second_phase`

This was causing simple examples to fail because the second phase of the
algorithm never kicked in. By moving the unmatching, this is avoided,
and is fine because of the corollaries in Irving's paper.

* Add example tests from #124

* Checkout CI file from dev.
daffidwilde added a commit that referenced this issue Oct 15, 2020
* Fix ipython version to avoid warnings. (#126)

* Fix ipython version to avoid warnings.

There were some weird warnings coming from an internal
`ipython`/`ipykernel` API, causing `pytest-nbval` to fail its tests. So,
downgrading `ipython` to 7.10 until that is fixed.

Details are available under
[this issue](ipython/ipykernel#540).

* Fix ipython version in workflow file.

* Move unmatching inside `second_phase` of SR (#125)

* Move unmatching inside `second_phase`

This was causing simple examples to fail because the second phase of the
algorithm never kicked in. By moving the unmatching, this is avoided,
and is fine because of the corollaries in Irving's paper.

* Add example tests from #124

* Checkout CI file from dev.
@daffidwilde
Copy link
Owner

Hi @MartinMohlenkamp, I've implemented the changes you've suggested here and released a new version on PyPI

I'll close the issue now, but if anything else comes up then please let me know! Thanks again

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

No branches or pull requests

2 participants