From 7981530dc0cd017f1998d08887429de556728e11 Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Thu, 16 Aug 2018 22:54:26 -0700 Subject: [PATCH 01/12] Add automatic allocation of copies of the share library on demand for multiple copies of players. --- src/axelrod_fortran/player.py | 124 ++++++++++++++++++++++++++++++---- tests/test_player.py | 20 +++++- tests/test_titfortat.py | 7 ++ 3 files changed, 134 insertions(+), 17 deletions(-) diff --git a/src/axelrod_fortran/player.py b/src/axelrod_fortran/player.py index 2237c11..82f20d9 100644 --- a/src/axelrod_fortran/player.py +++ b/src/axelrod_fortran/player.py @@ -1,10 +1,16 @@ +from collections import defaultdict +from ctypes import cdll, c_int, c_float, byref, POINTER +from ctypes.util import find_library +import os import random +import shutil +import tempfile +import uuid import warnings import axelrod as axl from axelrod.interaction_utils import compute_final_score from axelrod.action import Action -from ctypes import cdll, c_int, c_float, byref, POINTER from .strategies import characteristics C, D = Action.C, Action.D @@ -12,9 +18,97 @@ original_actions = {C: 0, D: 1} +self_interaction_message = """ +You are playing a match with the same player against itself. However +axelrod_fortran players share memory. You can initialise another instance of an +Axelrod_fortran player with player.clone(). +""" + + +class LibraryManager(object): + """LibraryManager creates and loads copies of a shared library, which + enables multiple copies of the same strategy to be run without the end user + having to maintain many copies of the shared library. + + This works by making a copy of the shared library file and loading it into + memory again. Loading the same file again will return a reference to the + same memory addresses. + + Additionally, library manager tracks how many copies of the library have + been loaded, and how many copies there are of each Player, so as to load + only as many copies of the shared library as needed. + """ + + def __init__(self, shared_library_name, verbose=False): + self.shared_library_name = shared_library_name + self.verbose = verbose + self.library_copies = [] + self.player_indices = defaultdict(set) + self.player_next = defaultdict(set) + # Generate a random prefix for tempfile generation + self.prefix = str(uuid.uuid4()) + self.library_path = self.find_shared_library(shared_library_name) + + def find_shared_library(self, shared_library_name): + ## This finds only the relative path to the library, unfortunately. + # reduced_name = shared_library_name.replace("lib", "").replace(".so", "") + # self.library_path = find_library(reduced_name) + # Hard code absolute path for testing purposes. + return "/usr/lib/libstrategies.so" + + def load_dll_copy(self): + # Copy the library file to a new location so we can load the copy. + temp_directory = tempfile.gettempdir() + copy_number = len(self.library_copies) + new_filename = os.path.join( + temp_directory, + "{}-{}-{}".format( + self.prefix, + str(copy_number), + self.shared_library_name) + ) + if self.verbose: + print("Loading {}".format(new_filename)) + shutil.copy2(self.library_path, new_filename) + shared_library = cdll.LoadLibrary(new_filename) + self.library_copies.append(shared_library) + + def next_player_index(self, name): + """Determine the index of the next free shared library copy to + allocate for the player. If none is available then load another copy.""" + # Is there a free index? + if len(self.player_next[name]) > 0: + return self.player_next[name].pop() + # Do we need to load a new copy? + player_count = len(self.player_indices[name]) + if player_count == len(self.library_copies): + self.load_dll_copy() + return player_count + # Find the first unused index + for i in range(len(self.library_copies)): + if i not in self.player_indices[name]: + return i + raise ValueError("We shouldn't be here.") + + def load_library_for_player(self, name): + index = self.next_player_index(name) + self.player_indices[name].add(index) + if self.verbose: + print("allocating {}".format(index)) + return index, self.library_copies[index] + + def release(self, name, index): + """Release the copy of the library so that it can be re-allocated.""" + self.player_indices[name].remove(index) + if self.verbose: + print("releasing {}".format(index)) + self.player_next[name].add(index) + + class Player(axl.Player): classifier = {"stochastic": True} + library_manager = None def __init__(self, original_name, shared_library_name='libstrategies.so'): @@ -27,9 +121,11 @@ def __init__(self, original_name, game: axelrod.Game A instance of an axelrod Game """ + if not Player.library_manager: + Player.library_manager = LibraryManager(shared_library_name) super().__init__() - self.shared_library_name = shared_library_name - self.shared_library = cdll.LoadLibrary(shared_library_name) + self.index, self.shared_library = \ + self.library_manager.load_library_for_player(original_name) self.original_name = original_name self.original_function = self.original_name is_stochastic = characteristics[self.original_name]['stochastic'] @@ -75,17 +171,8 @@ def original_strategy( return self.original_function(*[byref(arg) for arg in args]) def strategy(self, opponent): - if type(opponent) is Player \ - and (opponent.original_name == self.original_name) \ - and (opponent.shared_library_name == self.shared_library_name): - - message = """ -You are playing a match with two copies of the same player. -However the axelrod fortran players share memory. -You can initialise an instance of an Axelrod_fortran player with a -`shared_library_name` -variable that points to a copy of the shared library.""" - warnings.warn(message=message) + if self is opponent: + warnings.warn(message=self_interaction_message) if not self.history: their_last_move = 0 @@ -107,5 +194,14 @@ def strategy(self, opponent): return actions[original_action] def reset(self): + # Release the library before rest, which regenerates the player. + self.library_manager.release(self.original_name, self.index) super().reset() self.original_function = self.original_name + + def __del__(self): + # Release the library before deletion. + self.library_manager.release(self.original_name, self.index) + + def __repr__(self): + return self.original_name diff --git a/tests/test_player.py b/tests/test_player.py index 9d497d4..ecd454e 100644 --- a/tests/test_player.py +++ b/tests/test_player.py @@ -1,6 +1,6 @@ from axelrod_fortran import Player, characteristics, all_strategies -from axelrod import (Alternator, Cooperator, Defector, - Match, Game, basic_strategies, seed) +from axelrod import (Alternator, Cooperator, Defector, Match, MoranProcess, + Game, basic_strategies, seed) from axelrod.action import Action from ctypes import c_int, c_float, POINTER, CDLL @@ -26,6 +26,7 @@ def test_init(): assert type(player.shared_library) is CDLL assert "libstrategies.so" in str(player.shared_library) + def test_init_with_shared(): player = Player("k42r", shared_library_name="libstrategies.so") assert "libstrategies.so" == player.shared_library_name @@ -106,6 +107,7 @@ def test_original_strategy(): my_score += scores[0] their_score += scores[1] + def test_deterministic_strategies(): """ Test that the strategies classified as deterministic indeed act @@ -139,6 +141,7 @@ def test_implemented_strategies(): axl_match = Match((axl_player, opponent)) assert interactions == axl_match.play(), (player, opponent) + def test_champion_v_alternator(): """ Specific regression test for a bug. @@ -155,17 +158,20 @@ def test_champion_v_alternator(): seed(0) assert interactions == match.play() + def test_warning_for_self_interaction(recwarn): """ Test that a warning is given for a self interaction. """ player = Player("k42r") opponent = Player("k42r") + opponent = player match = Match((player, opponent)) interactions = match.play() - assert len(recwarn) == 1 + assert len(recwarn) == 0 + def test_no_warning_for_normal_interaction(recwarn): """ @@ -180,3 +186,11 @@ def test_no_warning_for_normal_interaction(recwarn): interactions = match.play() assert len(recwarn) == 0 + + +def test_multiple_copies(recwarn): + players = [Player('ktitfortatc') for _ in range(5)] + [ + Player('k42r') for _ in range(5)] + mp = MoranProcess(players) + mp.play() + mp.populations_plot() diff --git a/tests/test_titfortat.py b/tests/test_titfortat.py index b70160e..e13b425 100644 --- a/tests/test_titfortat.py +++ b/tests/test_titfortat.py @@ -24,3 +24,10 @@ def test_versus_defector(): match = axl.Match(players, 5) expected = [(C, D), (D, D), (D, D), (D, D), (D, D)] assert match.play() == expected + + +def test_versus_itself(): + players = (Player('ktitfortatc'), Player('ktitfortatc')) + match = axl.Match(players, 5) + expected = [(C, C), (C, C), (C, C), (C, C), (C, C)] + assert match.play() == expected From daa74319fc59c9e8284e435fe89a7ea477d84925 Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Fri, 17 Aug 2018 17:18:40 -0700 Subject: [PATCH 02/12] On LibraryManager destruction, delete any temporary files generated. --- src/axelrod_fortran/player.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/axelrod_fortran/player.py b/src/axelrod_fortran/player.py index 82f20d9..76dd287 100644 --- a/src/axelrod_fortran/player.py +++ b/src/axelrod_fortran/player.py @@ -48,6 +48,7 @@ def __init__(self, shared_library_name, verbose=False): # Generate a random prefix for tempfile generation self.prefix = str(uuid.uuid4()) self.library_path = self.find_shared_library(shared_library_name) + self.filenames = [] def find_shared_library(self, shared_library_name): ## This finds only the relative path to the library, unfortunately. @@ -57,6 +58,7 @@ def find_shared_library(self, shared_library_name): return "/usr/lib/libstrategies.so" def load_dll_copy(self): + """Load a new copy of the shared library.""" # Copy the library file to a new location so we can load the copy. temp_directory = tempfile.gettempdir() copy_number = len(self.library_copies) @@ -70,6 +72,7 @@ def load_dll_copy(self): if self.verbose: print("Loading {}".format(new_filename)) shutil.copy2(self.library_path, new_filename) + self.filenames.append(new_filename) shared_library = cdll.LoadLibrary(new_filename) self.library_copies.append(shared_library) @@ -91,6 +94,8 @@ def next_player_index(self, name): raise ValueError("We shouldn't be here.") def load_library_for_player(self, name): + """For a given player return a copy of the shared library for use + in a Player class, along with an index for later releasing.""" index = self.next_player_index(name) self.player_indices[name].add(index) if self.verbose: @@ -104,6 +109,12 @@ def release(self, name, index): print("releasing {}".format(index)) self.player_next[name].add(index) + def __del__(self): + """Cleanup temp files on object deletion.""" + for filename in self.filenames: + if os.path.exists(filename): + os.remove(filename) + class Player(axl.Player): @@ -121,13 +132,14 @@ def __init__(self, original_name, game: axelrod.Game A instance of an axelrod Game """ + super().__init__() if not Player.library_manager: Player.library_manager = LibraryManager(shared_library_name) - super().__init__() self.index, self.shared_library = \ self.library_manager.load_library_for_player(original_name) self.original_name = original_name self.original_function = self.original_name + is_stochastic = characteristics[self.original_name]['stochastic'] if is_stochastic is not None: self.classifier['stochastic'] = is_stochastic From 0b2e8dae3eee82d1ff502ce5ab7fdc964a89113d Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Fri, 17 Aug 2018 21:51:45 -0700 Subject: [PATCH 03/12] Dynamically located shared library path --- src/axelrod_fortran/player.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/axelrod_fortran/player.py b/src/axelrod_fortran/player.py index 76dd287..c8b411f 100644 --- a/src/axelrod_fortran/player.py +++ b/src/axelrod_fortran/player.py @@ -2,8 +2,11 @@ from ctypes import cdll, c_int, c_float, byref, POINTER from ctypes.util import find_library import os +import platform import random +import re import shutil +import subprocess import tempfile import uuid import warnings @@ -18,6 +21,8 @@ original_actions = {C: 0, D: 1} +path_regex = r""".*?\s=>\s(.*?{}.*?)\\""" + self_interaction_message = """ You are playing a match with the same player against itself. However axelrod_fortran players share memory. You can initialise another instance of an @@ -51,11 +56,17 @@ def __init__(self, shared_library_name, verbose=False): self.filenames = [] def find_shared_library(self, shared_library_name): - ## This finds only the relative path to the library, unfortunately. - # reduced_name = shared_library_name.replace("lib", "").replace(".so", "") - # self.library_path = find_library(reduced_name) - # Hard code absolute path for testing purposes. - return "/usr/lib/libstrategies.so" + # Hack for Linux since find_library doesn't return the full path. + if 'Linux' in platform.system(): + output = subprocess.check_output(["ldconfig", "-p"]) + for line in str(output).split(r"\n"): + rhs = line.split(" => ")[-1] + if shared_library_name in rhs: + return rhs + raise ValueError("{} not found".format(shared_library_name)) + else: + return find_library( + shared_library_name.replace("lib", "").replace(".so", "")) def load_dll_copy(self): """Load a new copy of the shared library.""" From 153b2276bb709ebc20412e95faf89160529a48ca Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Sat, 18 Aug 2018 19:56:22 -0700 Subject: [PATCH 04/12] Refactor shared library manager to be thread-safe and reorganize code --- src/axelrod_fortran/player.py | 139 +++--------------- src/axelrod_fortran/shared_library_manager.py | 121 +++++++++++++++ tests/test_player.py | 8 +- 3 files changed, 147 insertions(+), 121 deletions(-) create mode 100644 src/axelrod_fortran/shared_library_manager.py diff --git a/src/axelrod_fortran/player.py b/src/axelrod_fortran/player.py index c8b411f..f85c431 100644 --- a/src/axelrod_fortran/player.py +++ b/src/axelrod_fortran/player.py @@ -1,28 +1,19 @@ -from collections import defaultdict -from ctypes import cdll, c_int, c_float, byref, POINTER -from ctypes.util import find_library -import os -import platform +from ctypes import byref, c_float, c_int, POINTER import random -import re -import shutil -import subprocess -import tempfile -import uuid import warnings import axelrod as axl from axelrod.interaction_utils import compute_final_score from axelrod.action import Action + from .strategies import characteristics +from .shared_library_manager import MultiprocessManager, load_library C, D = Action.C, Action.D actions = {0: C, 1: D} original_actions = {C: 0, D: 1} -path_regex = r""".*?\s=>\s(.*?{}.*?)\\""" - self_interaction_message = """ You are playing a match with the same player against itself. However axelrod_fortran players share memory. You can initialise another instance of an @@ -30,110 +21,17 @@ """ -class LibraryManager(object): - """LibraryManager creates and loads copies of a shared library, which - enables multiple copies of the same strategy to be run without the end user - having to maintain many copies of the shared library. - - This works by making a copy of the shared library file and loading it into - memory again. Loading the same file again will return a reference to the - same memory addresses. - - Additionally, library manager tracks how many copies of the library have - been loaded, and how many copies there are of each Player, so as to load - only as many copies of the shared library as needed. - """ - - def __init__(self, shared_library_name, verbose=False): - self.shared_library_name = shared_library_name - self.verbose = verbose - self.library_copies = [] - self.player_indices = defaultdict(set) - self.player_next = defaultdict(set) - # Generate a random prefix for tempfile generation - self.prefix = str(uuid.uuid4()) - self.library_path = self.find_shared_library(shared_library_name) - self.filenames = [] - - def find_shared_library(self, shared_library_name): - # Hack for Linux since find_library doesn't return the full path. - if 'Linux' in platform.system(): - output = subprocess.check_output(["ldconfig", "-p"]) - for line in str(output).split(r"\n"): - rhs = line.split(" => ")[-1] - if shared_library_name in rhs: - return rhs - raise ValueError("{} not found".format(shared_library_name)) - else: - return find_library( - shared_library_name.replace("lib", "").replace(".so", "")) - - def load_dll_copy(self): - """Load a new copy of the shared library.""" - # Copy the library file to a new location so we can load the copy. - temp_directory = tempfile.gettempdir() - copy_number = len(self.library_copies) - new_filename = os.path.join( - temp_directory, - "{}-{}-{}".format( - self.prefix, - str(copy_number), - self.shared_library_name) - ) - if self.verbose: - print("Loading {}".format(new_filename)) - shutil.copy2(self.library_path, new_filename) - self.filenames.append(new_filename) - shared_library = cdll.LoadLibrary(new_filename) - self.library_copies.append(shared_library) - - def next_player_index(self, name): - """Determine the index of the next free shared library copy to - allocate for the player. If none is available then load another copy.""" - # Is there a free index? - if len(self.player_next[name]) > 0: - return self.player_next[name].pop() - # Do we need to load a new copy? - player_count = len(self.player_indices[name]) - if player_count == len(self.library_copies): - self.load_dll_copy() - return player_count - # Find the first unused index - for i in range(len(self.library_copies)): - if i not in self.player_indices[name]: - return i - raise ValueError("We shouldn't be here.") - - def load_library_for_player(self, name): - """For a given player return a copy of the shared library for use - in a Player class, along with an index for later releasing.""" - index = self.next_player_index(name) - self.player_indices[name].add(index) - if self.verbose: - print("allocating {}".format(index)) - return index, self.library_copies[index] - - def release(self, name, index): - """Release the copy of the library so that it can be re-allocated.""" - self.player_indices[name].remove(index) - if self.verbose: - print("releasing {}".format(index)) - self.player_next[name].add(index) - - def __del__(self): - """Cleanup temp files on object deletion.""" - for filename in self.filenames: - if os.path.exists(filename): - os.remove(filename) +# Initialize a module-wide manager for loading copies of the shared library. +manager = MultiprocessManager() +manager.start() +shared_library_manager = manager.SharedLibraryManager("libstrategies.so") class Player(axl.Player): classifier = {"stochastic": True} - library_manager = None - def __init__(self, original_name, - shared_library_name='libstrategies.so'): + def __init__(self, original_name): """ Parameters ---------- @@ -144,17 +42,16 @@ def __init__(self, original_name, A instance of an axelrod Game """ super().__init__() - if not Player.library_manager: - Player.library_manager = LibraryManager(shared_library_name) - self.index, self.shared_library = \ - self.library_manager.load_library_for_player(original_name) + self.index, self.shared_library_filename = \ + shared_library_manager.get_filename_for_player(original_name) + self.shared_library = load_library(self.shared_library_filename) self.original_name = original_name self.original_function = self.original_name - is_stochastic = characteristics[self.original_name]['stochastic'] if is_stochastic is not None: self.classifier['stochastic'] = is_stochastic + def __enter__(self): return self @@ -216,15 +113,21 @@ def strategy(self, opponent): my_last_move) return actions[original_action] + def _release_shared_library(self): + try: + shared_library_manager.release(self.original_name, self.index) + except FileNotFoundError: + pass + def reset(self): - # Release the library before rest, which regenerates the player. - self.library_manager.release(self.original_name, self.index) + # Release the shared library since the object is rebuilt on reset. + self._release_shared_library() super().reset() self.original_function = self.original_name def __del__(self): # Release the library before deletion. - self.library_manager.release(self.original_name, self.index) + self._release_shared_library() def __repr__(self): return self.original_name diff --git a/src/axelrod_fortran/shared_library_manager.py b/src/axelrod_fortran/shared_library_manager.py new file mode 100644 index 0000000..009af14 --- /dev/null +++ b/src/axelrod_fortran/shared_library_manager.py @@ -0,0 +1,121 @@ +from collections import defaultdict +from ctypes import cdll +from ctypes.util import find_library +from multiprocessing.managers import BaseManager +import os +import platform +import shutil +import subprocess +import tempfile +import uuid + + +def load_library(filename): + """Loads a shared library.""" + lib = None + if os.path.exists(filename): + lib = cdll.LoadLibrary(filename) + return lib + + +class SharedLibraryManager(object): + """LibraryManager creates (and deletes) copies of a shared library, which + enables multiple copies of the same strategy to be run without the end user + having to maintain many copies of the shared library. + + This works by making a copy of the shared library file and loading it into + memory again. Loading the same file again will return a reference to the + same memory addresses. To be thread-safe, this class just passes filenames + back to the Player class (which actually loads a reference to the library), + ensuring that multiple copies of a given player type do not use the same + copy of the shared library. + """ + + def __init__(self, shared_library_name, verbose=False): + self.shared_library_name = shared_library_name + self.verbose = verbose + self.filenames = [] + self.player_indices = defaultdict(set) + self.player_next = defaultdict(set) + # Generate a random prefix for tempfile generation + self.prefix = str(uuid.uuid4()) + self.library_path = self.find_shared_library(shared_library_name) + + def find_shared_library(self, shared_library_name): + # Hack for Linux since find_library doesn't return the full path. + if 'Linux' in platform.system(): + output = subprocess.check_output(["ldconfig", "-p"]) + for line in str(output).split(r"\n"): + rhs = line.split(" => ")[-1] + if shared_library_name in rhs: + return rhs + raise ValueError("{} not found".format(shared_library_name)) + else: + return find_library( + shared_library_name.replace("lib", "").replace(".so", "")) + + def load_dll_copy(self): + """Load a new copy of the shared library.""" + # Copy the library file to a new location so we can load the copy. + temp_directory = tempfile.gettempdir() + copy_number = len(self.filenames) + new_filename = os.path.join( + temp_directory, + "{}-{}-{}".format( + self.prefix, + str(copy_number), + self.shared_library_name) + ) + if self.verbose: + print("Loading {}".format(new_filename)) + shutil.copy2(self.library_path, new_filename) + self.filenames.append(new_filename) + + def next_player_index(self, name): + """Determine the index of the next free shared library copy to + allocate for the player. If none is available then load another copy.""" + # Is there a free index? + if len(self.player_next[name]) > 0: + return self.player_next[name].pop() + # Do we need to load a new copy? + player_count = len(self.player_indices[name]) + if player_count == len(self.filenames): + self.load_dll_copy() + return player_count + # Find the first unused index + for i in range(len(self.filenames)): + if i not in self.player_indices[name]: + return i + raise ValueError("We shouldn't be here.") + + def get_filename_for_player(self, name): + """For a given player return a copy of the shared library for use + in a Player class, along with an index for later releasing.""" + index = self.next_player_index(name) + self.player_indices[name].add(index) + if self.verbose: + print("allocating {}".format(index)) + return index, self.filenames[index] + + def release(self, name, index): + """Release the copy of the library so that it can be re-allocated.""" + self.player_indices[name].remove(index) + if self.verbose: + print("releasing {}".format(index)) + self.player_next[name].add(index) + + def __del__(self): + """Cleanup temp files on object deletion.""" + for filename in self.filenames: + if os.path.exists(filename): + if self.verbose: + print("deleting", filename) + os.remove(filename) + + +# Setup up thread safe library manager. +class MultiprocessManager(BaseManager): + pass + + +MultiprocessManager.register('SharedLibraryManager', SharedLibraryManager) diff --git a/tests/test_player.py b/tests/test_player.py index ecd454e..60012fa 100644 --- a/tests/test_player.py +++ b/tests/test_player.py @@ -1,11 +1,13 @@ +from ctypes import c_int, c_float, POINTER, CDLL +import itertools + +import pytest + from axelrod_fortran import Player, characteristics, all_strategies from axelrod import (Alternator, Cooperator, Defector, Match, MoranProcess, Game, basic_strategies, seed) from axelrod.action import Action -from ctypes import c_int, c_float, POINTER, CDLL -import itertools -import pytest C, D = Action.C, Action.D From 3a909cbb0f801ff2bf3e8f2bff68cf6be18ebc50 Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Sat, 18 Aug 2018 20:18:03 -0700 Subject: [PATCH 05/12] Update docstrings --- src/axelrod_fortran/shared_library_manager.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/axelrod_fortran/shared_library_manager.py b/src/axelrod_fortran/shared_library_manager.py index 009af14..738a3f1 100644 --- a/src/axelrod_fortran/shared_library_manager.py +++ b/src/axelrod_fortran/shared_library_manager.py @@ -54,9 +54,9 @@ def find_shared_library(self, shared_library_name): return find_library( shared_library_name.replace("lib", "").replace(".so", "")) - def load_dll_copy(self): - """Load a new copy of the shared library.""" - # Copy the library file to a new location so we can load the copy. + def create_library_copy(self): + """Create a new copy of the shared library.""" + # Copy the library file to a new (temp) location. temp_directory = tempfile.gettempdir() copy_number = len(self.filenames) new_filename = os.path.join( @@ -73,14 +73,14 @@ def load_dll_copy(self): def next_player_index(self, name): """Determine the index of the next free shared library copy to - allocate for the player. If none is available then load another copy.""" + allocate for the player. If none is available then make another copy.""" # Is there a free index? if len(self.player_next[name]) > 0: return self.player_next[name].pop() # Do we need to load a new copy? player_count = len(self.player_indices[name]) if player_count == len(self.filenames): - self.load_dll_copy() + self.create_library_copy() return player_count # Find the first unused index for i in range(len(self.filenames)): @@ -89,8 +89,8 @@ def next_player_index(self, name): raise ValueError("We shouldn't be here.") def get_filename_for_player(self, name): - """For a given player return a copy of the shared library for use - in a Player class, along with an index for later releasing.""" + """For a given player return a filename for a copy of the shared library + for use in a Player class, along with an index for later releasing.""" index = self.next_player_index(name) self.player_indices[name].add(index) if self.verbose: From dc43053bb6dd1640480c58cfcf65b87903832cbb Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Mon, 20 Aug 2018 22:45:19 -0700 Subject: [PATCH 06/12] Remove explicit references to shared library file in tests --- tests/test_player.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/test_player.py b/tests/test_player.py index 60012fa..54b57a2 100644 --- a/tests/test_player.py +++ b/tests/test_player.py @@ -24,16 +24,6 @@ def test_init(): assert player.original_function.restype == c_int with pytest.raises(ValueError): player = Player('test') - assert "libstrategies.so" == player.shared_library_name - assert type(player.shared_library) is CDLL - assert "libstrategies.so" in str(player.shared_library) - - -def test_init_with_shared(): - player = Player("k42r", shared_library_name="libstrategies.so") - assert "libstrategies.so" == player.shared_library_name - assert type(player.shared_library) is CDLL - assert "libstrategies.so" in str(player.shared_library) def test_matches(): From 299dd7e831452ec94da61aa2a2dddeb1be386acb Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Mon, 20 Aug 2018 22:51:24 -0700 Subject: [PATCH 07/12] Update travis config to make libstrategies.so locateable by ldconfig. --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4f4303f..0f25d2d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,9 @@ before_install: - git clone https://github.com/Axelrod-Python/TourExec.git /tmp/TourExec - cd /tmp/TourExec - sudo make install - - export LD_LIBRARY_PATH=/usr/local/lib + - echo "/usr/lib/libstrategies.so" | sudo tee /etc/ld.so.conf.d/strategies-lib.conf + - sudo ldconfig + - ldconfig -p | grep libstrategies.so - cd $TRAVIS_BUILD_DIR install: - pip install -r requirements.txt From 401acaf18809639ef00e7a632d863c1c7694604f Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Mon, 20 Aug 2018 22:57:35 -0700 Subject: [PATCH 08/12] Fix test --- tests/test_player.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_player.py b/tests/test_player.py index 54b57a2..8425697 100644 --- a/tests/test_player.py +++ b/tests/test_player.py @@ -156,13 +156,12 @@ def test_warning_for_self_interaction(recwarn): Test that a warning is given for a self interaction. """ player = Player("k42r") - opponent = Player("k42r") opponent = player match = Match((player, opponent)) interactions = match.play() - assert len(recwarn) == 0 + assert len(recwarn) == 1 def test_no_warning_for_normal_interaction(recwarn): From a2ab830536eb7a1493514182b5bba95cfe5343a7 Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Mon, 20 Aug 2018 23:26:46 -0700 Subject: [PATCH 09/12] Add comment to explain exception --- src/axelrod_fortran/player.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/axelrod_fortran/player.py b/src/axelrod_fortran/player.py index f85c431..e9612a7 100644 --- a/src/axelrod_fortran/player.py +++ b/src/axelrod_fortran/player.py @@ -114,6 +114,10 @@ def strategy(self, opponent): return actions[original_action] def _release_shared_library(self): + # While this looks like we're checking that the shared library file + # isn't deleted, the exception is actually thrown in the manager + # thread closes before the player class is garbage collected, which + # tends to happen at the end of a script. try: shared_library_manager.release(self.original_name, self.index) except FileNotFoundError: From 6d90fe53aec82e6d5c882ee8458d79e6f3cd0e04 Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Tue, 21 Aug 2018 07:15:40 -0700 Subject: [PATCH 10/12] Refactor file manipulations to use pathlib --- src/axelrod_fortran/player.py | 2 +- src/axelrod_fortran/shared_library_manager.py | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/axelrod_fortran/player.py b/src/axelrod_fortran/player.py index e9612a7..19198a7 100644 --- a/src/axelrod_fortran/player.py +++ b/src/axelrod_fortran/player.py @@ -115,7 +115,7 @@ def strategy(self, opponent): def _release_shared_library(self): # While this looks like we're checking that the shared library file - # isn't deleted, the exception is actually thrown in the manager + # isn't deleted, the exception is actually thrown if the manager # thread closes before the player class is garbage collected, which # tends to happen at the end of a script. try: diff --git a/src/axelrod_fortran/shared_library_manager.py b/src/axelrod_fortran/shared_library_manager.py index 738a3f1..f3a7e6d 100644 --- a/src/axelrod_fortran/shared_library_manager.py +++ b/src/axelrod_fortran/shared_library_manager.py @@ -3,6 +3,7 @@ from ctypes.util import find_library from multiprocessing.managers import BaseManager import os +from pathlib import Path import platform import shutil import subprocess @@ -59,13 +60,11 @@ def create_library_copy(self): # Copy the library file to a new (temp) location. temp_directory = tempfile.gettempdir() copy_number = len(self.filenames) - new_filename = os.path.join( - temp_directory, - "{}-{}-{}".format( - self.prefix, - str(copy_number), - self.shared_library_name) - ) + filename = "{}-{}-{}".format( + self.prefix, + str(copy_number), + self.shared_library_name) + new_filename = str(Path(temp_directory) / filename) if self.verbose: print("Loading {}".format(new_filename)) shutil.copy2(self.library_path, new_filename) @@ -107,10 +106,11 @@ def release(self, name, index): def __del__(self): """Cleanup temp files on object deletion.""" for filename in self.filenames: - if os.path.exists(filename): + path = Path(filename) + if path.exists(): if self.verbose: - print("deleting", filename) - os.remove(filename) + print("deleting", str(path)) + os.remove(str(path)) # Setup up thread safe library manager. From 235e59b9d908915b43fcdf11bcbc99c6bf8419db Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Tue, 21 Aug 2018 07:31:46 -0700 Subject: [PATCH 11/12] More refactor file manipulations to use pathlib --- src/axelrod_fortran/shared_library_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/axelrod_fortran/shared_library_manager.py b/src/axelrod_fortran/shared_library_manager.py index f3a7e6d..b60171f 100644 --- a/src/axelrod_fortran/shared_library_manager.py +++ b/src/axelrod_fortran/shared_library_manager.py @@ -14,7 +14,7 @@ def load_library(filename): """Loads a shared library.""" lib = None - if os.path.exists(filename): + if Path(filename).exists(): lib = cdll.LoadLibrary(filename) return lib @@ -64,7 +64,7 @@ def create_library_copy(self): self.prefix, str(copy_number), self.shared_library_name) - new_filename = str(Path(temp_directory) / filename) + new_filename = str(Path(temp_directory, filename)) if self.verbose: print("Loading {}".format(new_filename)) shutil.copy2(self.library_path, new_filename) @@ -110,7 +110,7 @@ def __del__(self): if path.exists(): if self.verbose: print("deleting", str(path)) - os.remove(str(path)) + path.unlink() # Setup up thread safe library manager. From 181c08c7993ec15aa6801fea764976b5719f47e1 Mon Sep 17 00:00:00 2001 From: Marc Harper Date: Tue, 21 Aug 2018 07:38:34 -0700 Subject: [PATCH 12/12] Remove unused import --- src/axelrod_fortran/shared_library_manager.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/axelrod_fortran/shared_library_manager.py b/src/axelrod_fortran/shared_library_manager.py index b60171f..8fadf6b 100644 --- a/src/axelrod_fortran/shared_library_manager.py +++ b/src/axelrod_fortran/shared_library_manager.py @@ -2,7 +2,6 @@ from ctypes import cdll from ctypes.util import find_library from multiprocessing.managers import BaseManager -import os from pathlib import Path import platform import shutil