Add support for copy.deepcopy on instances of RandObj

- Add tests that copying an object retains its determinism.
- Fix resulting bugs from that.
- Marginally improve MultiVar performance by changing a dict to a list.

Author: will-keen

constrainedrandom/internal/multivar.py

@@ -37,7 +37,7 @@ class MultiVarProblem:
     def __init__(
         self,
         parent: 'RandObj',
-        vars: Dict[str, 'RandVar'],
+        vars: List['RandVar'],
         constraints: Iterable[utils.ConstraintAndVars],
         max_iterations: int,
         max_domain_size: int,
@@ -71,15 +71,14 @@ def determine_order(self, with_values: Dict[str, Any]) -> List[List['RandVar']]:
         # to solve at the same time.
         # The best case is to simply solve them all at once, if possible, however it is
         # likely that the domain will be too large.
-        vars = []
-
         # If values are provided, simply don't add those variables to the ordering problem.
         if problem_changed:
-            for name, var in self.vars.items():
-                if name not in with_values:
+            vars = []
+            for var in self.vars:
+                if var.name not in with_values:
                     vars.append(var)
         else:
-            vars = list(self.vars.values())
+            vars = list(self.vars)
 
         # Use order hints first, remaining variables can be placed anywhere the domain
         # isn't too large.

constrainedrandom/internal/randvar.py

@@ -4,14 +4,37 @@
 import constraint
 from functools import partial
 from itertools import product
-from typing import Any, Callable, Iterable, Optional, Union
+from typing import Any, Callable, Iterable, List, Optional
 import random
 
 from .. import utils
 from ..debug import RandomizationDebugInfo, RandomizationFail
 from ..random import dist
 
 
+def get_and_call(getter: Callable, member_fn: str, *args: List[Any]):
+    '''
+    This is a very strange workaround for a very strange issue.
+    ``copy.deepcopy`` can handle a ``partial`` for all other members
+    of ``random.Random``, but not ``getrandbits``. I.e. it correctly
+    copies the other functions and their instance of ``random.Random``,
+    but not ``getrandbits``. The reason for this is unknown.
+
+    This function therefore exists to work around that issue
+    by getting ``getrandbits`` and calling it. I tried many
+    other approaches, but this was the only one that worked.
+
+    :param getter: Getter to call, returning an object that
+        has a member function with name ``member_fn``.
+    :param member_fn: Member function of the the object returned
+        by ``getter``.
+    :param args: Arguments to supply to ``member_fn``.
+    '''
+    callable_obj = getter()
+    fn = getattr(callable_obj, member_fn)
+    return fn(*args)
+
+
 class RandVar:
     '''
     Randomizable variable. For internal use with :class:`RandObj`.
@@ -99,6 +122,10 @@ def create_randomizer(self) -> Callable:
         We do this to create a more optimal randomizer than the user might
         have specified that is functionally equivalent.
 
+        We always return a ``partial`` because these work with
+        ``copy.deepcopy``, whereas locally-defined functions and
+        lambdas can only ever have one instance.
+
         :return: a function as described.
         :raises TypeError: if the domain is of a bad type.
         '''
@@ -111,7 +138,9 @@ def create_randomizer(self) -> Callable:
                 return self.fn
         elif self.bits is not None:
             self.domain = range(0, 1 << self.bits)
-            return partial(self._get_random().getrandbits, self.bits)
+            # This is still faster than doing self._get_random().randrange(self.bits << 1),
+            # it seems that getrandbits is 10x faster than randrange.
+            return partial(get_and_call, self._get_random, 'getrandbits', self.bits)
         else:
             # Handle possible types of domain.
             is_range = isinstance(self.domain, range)
@@ -146,10 +175,8 @@ def create_randomizer(self) -> Callable:
                     debug_info.add_failure(debug_fail)
                     raise utils.RandomizationError("Variable was unsolvable. Check constraints.", debug_info)
                 solution_list = [s[self.name] for s in solutions]
-                def solution_picker(solns):
-                    return self._get_random().choice(solns)
                 self.check_constraints = False
-                return partial(solution_picker, solution_list)
+                return partial(self._get_random().choice, solution_list)
             elif is_range:
                 return partial(self._get_random().randrange, self.domain.start, self.domain.stop)
             elif is_list_or_tuple:
@@ -308,7 +335,7 @@ def randomize_list_csp(
         problem = constraint.Problem()
         possible_values = self.get_constraint_domain()
         # Prune possibilities according to scalar constraints.
-        possible_values[:] = [x for x in possible_values \
+        possible_values = [x for x in possible_values \
             if all(constr(val) for val in x for constr in constraints)]
         problem.addVariable(self.name, possible_values)
         for con in list_constraints:

constrainedrandom/randobj.py

@@ -318,18 +318,20 @@ def randomize(
                     problem_changed = True
             # If a variable becomes constrained due to temporary multi-variable
             # constraints, we must respect single var temporary constraints too.
-            for var, constrs in tmp_single_var_constraints.items():
+            for var, constrs in sorted(tmp_single_var_constraints.items()):
                 if var in constrained_vars:
                     for constr in constrs:
                         constraints.append((constr, (var,)))
 
         # Don't allow non-determinism when iterating over a set
         constrained_vars = sorted(constrained_vars)
+        # Don't allow non-determinism when iterating over a dict
+        random_vars = sorted(self._random_vars.items())
 
         # Process concrete values - use these preferentially
         with_values = with_values if with_values is not None else {}
 
-        for name, random_var in self._random_vars.items():
+        for name, random_var in random_vars:
             if name in with_values:
                 result[name] = with_values[name]
             else:
@@ -379,7 +381,7 @@ def randomize(
             if problem_changed or self._problem_changed or self._multi_var_problem is None:
                 multi_var_problem = MultiVarProblem(
                     self,
-                    {name: var for name, var in self._random_vars.items() if name in constrained_vars},
+                    [var for var_name, var in random_vars if var_name in constrained_vars],
                     constraints,
                     max_iterations=self._max_iterations,
                     max_domain_size=self._max_domain_size,

constrainedrandom/random.py

@@ -8,7 +8,7 @@
 from . import utils
 
 
-def weighted_choice(choices_dict: utils.Dist, _random: Optional[random.Random]=random) -> Any:
+def weighted_choice(choices_dict: utils.Dist, _random: Optional[random.Random]=None) -> Any:
     '''
     Wrapper around ``random.choices``, allowing the user to specify weights in a dictionary.
 
@@ -24,9 +24,12 @@ def weighted_choice(choices_dict: utils.Dist, _random: Optional[random.Random]=r
         # 0 will be chosen 25% of the time, 1 25% of the time and 'foo' 50% of the time
         value = weighted_choice({0: 25, 1: 25, 'foo': 50})
     '''
+    if _random is None:
+        _random = random
     return _random.choices(tuple(choices_dict.keys()), weights=tuple(choices_dict.values()))
 
-def dist(dist_dict: utils.Dist, _random: Optional[random.Random]=random) -> Any:
+
+def dist(dist_dict: utils.Dist, _random: Optional[random.Random]=None) -> Any:
     '''
     Random distribution. As :func:`weighted_choice`, but allows ``range`` to be used as
     a key to the dictionary, which if chosen is then evaluated as a random range.
@@ -46,6 +49,8 @@ def dist(dist_dict: utils.Dist, _random: Optional[random.Random]=random) -> Any:
         # and 'foo' 50% of the time
         value = dist({0: 25, range(1, 10): 25, 'foo': 50})
     '''
+    if _random is None:
+        _random = random
     answer = weighted_choice(choices_dict=dist_dict, _random=_random)[0]
     if isinstance(answer, range):
         return _random.randrange(answer.start, answer.stop)

tests/testutils.py

@@ -5,6 +5,7 @@
 
 import timeit
 import unittest
+from copy import deepcopy
 from typing import Any, Dict, List
 
 from constrainedrandom import RandObj, RandomizationError
@@ -72,7 +73,7 @@ def tmp_check(self, results) -> None:
         '''
         pass
 
-    def randomize_and_time(self, randobj, iterations, tmp_constraints=None, tmp_values=None) -> Dict[str, Any]:
+    def randomize_and_time(self, randobj, iterations, tmp_constraints=None, tmp_values=None) -> List[Dict[str, Any]]:
         '''
         Call randobj.randomize() iterations times, time it, print performance stats,
         return the results.
@@ -93,7 +94,6 @@ def randomize_and_time(self, randobj, iterations, tmp_constraints=None, tmp_valu
         print(f'{self.get_full_test_name()} took {time_taken:.4g}s for {iterations} iterations ({hz:.1f}Hz)')
         return results
 
-
     def test_randobj(self) -> None:
         '''
         Reusable test function to randomize a RandObj for a number of iterations and perform checks.
@@ -109,6 +109,8 @@ def test_randobj(self) -> None:
 
         # Test with seed 0
         randobj = self.get_randobj(0)
+        # Take a copy of the randobj for use later
+        randobj_copy = deepcopy(randobj)
         if self.EXPECT_FAILURE:
             self.assertRaises(RandomizationError, randobj.randomize)
         else:
@@ -128,41 +130,43 @@ def test_randobj(self) -> None:
                     add_results = self.randomize_and_time(randobj, self.iterations, tmp_values=tmp_values)
                     self.tmp_check(add_results)
 
-        # Test again with seed 0, ensuring results are the same
+        # Test again with seed 0, ensuring results are the same.
+        # Also test the copy we took earlier.
         randobj0 = self.get_randobj(0)
-        if self.EXPECT_FAILURE:
-            self.assertRaises(RandomizationError, randobj0.randomize)
-        else:
-            results0 = self.randomize_and_time(randobj0, self.iterations)
-            assertListOfDictsEqual(self, results, results0, "Non-determinism detected, results were not equal")
-            if do_tmp_checks:
-                # Check applying temporary constraints is also deterministic
-                tmp_results0 = self.randomize_and_time(randobj0, self.iterations, tmp_constraints, tmp_values)
-                assertListOfDictsEqual(
-                    self,
-                    tmp_results,
-                    tmp_results0,
-                    "Non-determinism detected, results were not equal with temp constraints"
-                )
-                # Check temporary constraints don't break base randomization determinism
-                post_tmp_results0 = self.randomize_and_time(randobj0, self.iterations)
-                assertListOfDictsEqual(
-                    self,
-                    post_tmp_results,
-                    post_tmp_results0,
-                    "Non-determinism detected, results were not equal after temp constraints"
-                )
-                # Add temporary constraints permanently, see what happens
-                if tmp_constraints is not None:
-                    for constr, vars in tmp_constraints:
-                        randobj0.add_constraint(constr, vars)
-                    add_results0 = self.randomize_and_time(randobj0, self.iterations, tmp_values=tmp_values)
+        for tmp_randobj in [randobj0, randobj_copy]:
+            if self.EXPECT_FAILURE:
+                self.assertRaises(RandomizationError, tmp_randobj.randomize)
+            else:
+                results0 = self.randomize_and_time(tmp_randobj, self.iterations)
+                assertListOfDictsEqual(self, results, results0, "Non-determinism detected, results were not equal")
+                if do_tmp_checks:
+                    # Check applying temporary constraints is also deterministic
+                    tmp_results0 = self.randomize_and_time(tmp_randobj, self.iterations, tmp_constraints, tmp_values)
+                    assertListOfDictsEqual(
+                        self,
+                        tmp_results,
+                        tmp_results0,
+                        "Non-determinism detected, results were not equal with temp constraints"
+                    )
+                    # Check temporary constraints don't break base randomization determinism
+                    post_tmp_results0 = self.randomize_and_time(tmp_randobj, self.iterations)
                     assertListOfDictsEqual(
                         self,
-                        add_results,
-                        add_results0,
-                        "Non-determinism detected, results were not equal after constraints added"
+                        post_tmp_results,
+                        post_tmp_results0,
+                        "Non-determinism detected, results were not equal after temp constraints"
                     )
+                    # Add temporary constraints permanently, see what happens
+                    if tmp_constraints is not None:
+                        for constr, vars in tmp_constraints:
+                            tmp_randobj.add_constraint(constr, vars)
+                        add_results0 = self.randomize_and_time(tmp_randobj, self.iterations, tmp_values=tmp_values)
+                        assertListOfDictsEqual(
+                            self,
+                            add_results,
+                            add_results0,
+                            "Non-determinism detected, results were not equal after constraints added"
+                        )
 
         # Test with seed 1, ensuring results are different
         randobj1 = self.get_randobj(1)