daffidwilde / matching

A package for solving matching games
https://daffidwilde.github.io/matching/
MIT License
149 stars 42 forks source link

`deepcopy` in `StableMarriage` with ~270x270 Players -> Process finished with exit code -1073741571 (0xC00000FD) #135

Closed deponovo closed 3 years ago

deponovo commented 3 years ago

I bumped into a Process finished with exit code -1073741571 (0xC00000FD) error. This is caused by the deepcopy in the __init__ method of the StableMarriage class (matching.games.stable_marriage.py):

  def __init__(self, suitors, reviewers):

      suitors, reviewers = copy.deepcopy([suitors, reviewers])   # this is my own comment
      self.suitors = suitors
      self.reviewers = reviewers

      super().__init__()
      self.check_inputs()

While in debug mode, I placed a breakpoint at the line marked with my comment and tried even to deepcopy a single Player, e.g. deepcopy(suitors[0]). This also led to the same error.

The reason why this is happening is not clear to me. Also the need for the deepcopy is not clear and I implemented a work around this problem by using the functional implementations as (disclaimer: pseudo code):

from matching.games.stable_marriage import _make_players as make_stable_marriage_players
from matching.algorithms import stable_marriage

suitors, reviewers = make_stable_marriage_players(men, women)
match_indices = list(stable_marriage(suitors, reviewers).items())

Also, in a venv, I removed the deepcopy line of code commented above and the results of the matching were the same.

In the end I am reporting this because it seems to be an implementation bug. For my purpose I got a workaround solution as reported (which is not needing to make modifications to the package, which would be not the correct way to do it), but it could be that I am missing something here.

daffidwilde commented 3 years ago

Hi @deponovo, thanks for raising this issue, and sorry that it has taken this long to get back to you.

It's certainly a strange one. From digging around, that error looks like some sort of memory issue. I'm not sure how to remedy that exactly, but it appears to be a fairly common issue with PyCharm (if that's the IDE you're using).

For context, the reason I have that deepcopy in there is so the original instances of Player are unaffected by running the game. That way, multiple games can be run without having to create the instances each time. That way, the behaviour is the same as if you created the game using dictionaries.

Anyway, I'll have a play around with your workaround to see if it messes with the test suite. I am unable to recreate the error myself, so I would appreciate it if you could run through everything when the PR is open (I'll ping you).

However, I doubt this workaround will work in general as it only creates shallow copies. We will see!