From 396ed34c047078a5414ba42135df6bb4dd873f03 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Tue, 14 Apr 2020 16:55:18 +0200 Subject: [PATCH 1/9] pass the sequence with the function, fixes https://github.com/python-adaptive/adaptive/issues/265 --- adaptive/learner/sequence_learner.py | 42 ++++++++++++---------------- 1 file changed, 18 insertions(+), 24 deletions(-) diff --git a/adaptive/learner/sequence_learner.py b/adaptive/learner/sequence_learner.py index c7398dfa4..0f9aa1b38 100644 --- a/adaptive/learner/sequence_learner.py +++ b/adaptive/learner/sequence_learner.py @@ -5,8 +5,8 @@ from adaptive.learner.base_learner import BaseLearner -class _IgnoreFirstArgument: - """Remove the first argument from the call signature. +class _IndexToPoint: + """Call function with index of sequence. The SequenceLearner's function receives a tuple ``(index, point)`` but the original function only takes ``point``. @@ -15,18 +15,19 @@ class _IgnoreFirstArgument: pickable. """ - def __init__(self, function): + def __init__(self, function, sequence): self.function = function + self.sequence = sequence - def __call__(self, index_point, *args, **kwargs): - index, point = index_point + def __call__(self, index, *args, **kwargs): + point = self.sequence[index] return self.function(point, *args, **kwargs) def __getstate__(self): - return self.function + return self.function, self.sequence - def __setstate__(self, function): - self.__init__(function) + def __setstate__(self, state): + self.__init__(*state) class SequenceLearner(BaseLearner): @@ -40,7 +41,7 @@ class SequenceLearner(BaseLearner): Parameters ---------- function : callable - The function to learn. Must take a single element `sequence`. + The function to learn. Must take a single element of `sequence`. sequence : sequence The sequence to learn. @@ -58,7 +59,7 @@ class SequenceLearner(BaseLearner): def __init__(self, function, sequence): self._original_function = function - self.function = _IgnoreFirstArgument(function) + self.function = _IndexToPoint(function, sequence) self._to_do_indices = SortedSet({i for i, _ in enumerate(sequence)}) self._ntotal = len(sequence) self.sequence = copy(sequence) @@ -67,21 +68,18 @@ def __init__(self, function, sequence): def ask(self, n, tell_pending=True): indices = [] - points = [] loss_improvements = [] for index in self._to_do_indices: - if len(points) >= n: + if len(indices) >= n: break - point = self.sequence[index] indices.append(index) - points.append((index, point)) loss_improvements.append(1 / self._ntotal) if tell_pending: - for i, p in zip(indices, points): - self.tell_pending((i, p)) + for index in indices: + self.tell_pending(index) - return points, loss_improvements + return indices, loss_improvements def _get_data(self): return self.data @@ -89,9 +87,7 @@ def _get_data(self): def _set_data(self, data): if data: indices, values = zip(*data.items()) - # the points aren't used by tell, so we can safely pass None - points = [(i, None) for i in indices] - self.tell_many(points, values) + self.tell_many(indices, values) def loss(self, real=True): if not (self._to_do_indices or self.pending_points): @@ -105,14 +101,12 @@ def remove_unfinished(self): self._to_do_indices.add(i) self.pending_points = set() - def tell(self, point, value): - index, point = point + def tell(self, index, value): self.data[index] = value self.pending_points.discard(index) self._to_do_indices.discard(index) - def tell_pending(self, point): - index, point = point + def tell_pending(self, index): self.pending_points.add(index) self._to_do_indices.discard(index) From 91990fd026e61dabda8d933c940f3f5637c490e5 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Tue, 14 Apr 2020 17:04:30 +0200 Subject: [PATCH 2/9] add test_fail_with_sequence_of_unhashable --- adaptive/tests/test_sequence_learner.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 adaptive/tests/test_sequence_learner.py diff --git a/adaptive/tests/test_sequence_learner.py b/adaptive/tests/test_sequence_learner.py new file mode 100644 index 000000000..d503fdbfe --- /dev/null +++ b/adaptive/tests/test_sequence_learner.py @@ -0,0 +1,24 @@ +import asyncio + +import numpy as np + +import adaptive + + +def might_fail(dct): + import random + + if random.random() < 0.5: + raise Exception() + return dct["x"] + + +def test_fail_with_sequence_of_unhashable(): + # https://github.com/python-adaptive/adaptive/issues/265 + seq = [dict(x=x) for x in np.linspace(-1, 1, 101)] # unhashable + learner = adaptive.SequenceLearner(might_fail, sequence=seq) + runner = adaptive.Runner( + learner, goal=adaptive.SequenceLearner.done, retries=100 + ) # with 100 retries the test will fail once in 10^31 + asyncio.get_event_loop().run_until_complete(runner.task) + assert runner.status() == "finished" From 093f148f0dbcaa182a489d8304eafcfc7891e3dd Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Tue, 14 Apr 2020 17:53:01 +0200 Subject: [PATCH 3/9] no not treat the SequenceLearner specially --- adaptive/tests/test_learners.py | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/adaptive/tests/test_learners.py b/adaptive/tests/test_learners.py index 6edee2b30..dc7f11eb2 100644 --- a/adaptive/tests/test_learners.py +++ b/adaptive/tests/test_learners.py @@ -281,19 +281,9 @@ def test_adding_existing_data_is_idempotent(learner_type, f, learner_kwargs): M = random.randint(10, 30) pls = zip(*learner.ask(M)) cpls = zip(*control.ask(M)) - if learner_type is SequenceLearner: - # The SequenceLearner's points might not be hasable - points, values = zip(*pls) - indices, points = zip(*points) - cpoints, cvalues = zip(*cpls) - cindices, cpoints = zip(*cpoints) - assert (np.array(points) == np.array(cpoints)).all() - assert values == cvalues - assert indices == cindices - else: - # Point ordering is not defined, so compare as sets - assert set(pls) == set(cpls) + # Point ordering is not defined, so compare as sets + assert set(pls) == set(cpls) # XXX: This *should* pass (https://github.com/python-adaptive/adaptive/issues/55) @@ -324,20 +314,9 @@ def test_adding_non_chosen_data(learner_type, f, learner_kwargs): pls = zip(*learner.ask(M)) cpls = zip(*control.ask(M)) - if learner_type is SequenceLearner: - # The SequenceLearner's points might not be hasable - points, values = zip(*pls) - indices, points = zip(*points) - - cpoints, cvalues = zip(*cpls) - cindices, cpoints = zip(*cpoints) - assert (np.array(points) == np.array(cpoints)).all() - assert values == cvalues - assert indices == cindices - else: - # Point ordering within a single call to 'ask' - # is not guaranteed to be the same by the API. - assert set(pls) == set(cpls) + # Point ordering within a single call to 'ask' + # is not guaranteed to be the same by the API. + assert set(pls) == set(cpls) @run_with(Learner1D, xfail(Learner2D), xfail(LearnerND), AverageLearner) From ccdbe766174ba484a4a0840005bc9c9933c6e318 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Thu, 16 Apr 2020 16:47:34 +0200 Subject: [PATCH 4/9] make __call__ a one-liner Co-Authored-By: Anton Akhmerov --- adaptive/learner/sequence_learner.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/adaptive/learner/sequence_learner.py b/adaptive/learner/sequence_learner.py index 0f9aa1b38..40ace9d8c 100644 --- a/adaptive/learner/sequence_learner.py +++ b/adaptive/learner/sequence_learner.py @@ -20,8 +20,7 @@ def __init__(self, function, sequence): self.sequence = sequence def __call__(self, index, *args, **kwargs): - point = self.sequence[index] - return self.function(point, *args, **kwargs) + return self.function(self.sequence[index], *args, **kwargs) def __getstate__(self): return self.function, self.sequence From 35930c7a7ae7c6dc19d788f7ec5ef4a569961001 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Thu, 16 Apr 2020 16:47:58 +0200 Subject: [PATCH 5/9] remove __getstate__ and __setstate__ in _IndexToPoint --- adaptive/learner/sequence_learner.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/adaptive/learner/sequence_learner.py b/adaptive/learner/sequence_learner.py index 40ace9d8c..890253a76 100644 --- a/adaptive/learner/sequence_learner.py +++ b/adaptive/learner/sequence_learner.py @@ -22,12 +22,6 @@ def __init__(self, function, sequence): def __call__(self, index, *args, **kwargs): return self.function(self.sequence[index], *args, **kwargs) - def __getstate__(self): - return self.function, self.sequence - - def __setstate__(self, state): - self.__init__(*state) - class SequenceLearner(BaseLearner): r"""A learner that will learn a sequence. It simply returns From 02f69be8d501b64c60395086154caec0d122d5a7 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Thu, 16 Apr 2020 17:20:10 +0200 Subject: [PATCH 6/9] remove obsolete doc-string --- adaptive/learner/sequence_learner.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/adaptive/learner/sequence_learner.py b/adaptive/learner/sequence_learner.py index 890253a76..0069f53e2 100644 --- a/adaptive/learner/sequence_learner.py +++ b/adaptive/learner/sequence_learner.py @@ -6,14 +6,7 @@ class _IndexToPoint: - """Call function with index of sequence. - - The SequenceLearner's function receives a tuple ``(index, point)`` - but the original function only takes ``point``. - - This is the same as `lambda x: function(x[1])`, however, that is not - pickable. - """ + """Call function with index of sequence.""" def __init__(self, function, sequence): self.function = function From 7cf2447e557ec6eb2e137ce2e41178f24a8d51db Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Thu, 16 Apr 2020 17:24:21 +0200 Subject: [PATCH 7/9] define and use a FailOnce class that is callable --- adaptive/tests/test_sequence_learner.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/adaptive/tests/test_sequence_learner.py b/adaptive/tests/test_sequence_learner.py index d503fdbfe..6cc06e963 100644 --- a/adaptive/tests/test_sequence_learner.py +++ b/adaptive/tests/test_sequence_learner.py @@ -2,23 +2,27 @@ import numpy as np -import adaptive +from adaptive import Runner, SequenceLearner +from adaptive.runner import SequentialExecutor -def might_fail(dct): - import random +class FailOnce: + def __init__(self): + self.failed = False - if random.random() < 0.5: - raise Exception() - return dct["x"] + def __call__(self, value): + if self.failed: + return value + self.failed = True + raise Exception def test_fail_with_sequence_of_unhashable(): # https://github.com/python-adaptive/adaptive/issues/265 seq = [dict(x=x) for x in np.linspace(-1, 1, 101)] # unhashable - learner = adaptive.SequenceLearner(might_fail, sequence=seq) - runner = adaptive.Runner( - learner, goal=adaptive.SequenceLearner.done, retries=100 + learner = SequenceLearner(FailOnce(), sequence=seq) + runner = Runner( + learner, goal=SequenceLearner.done, retries=100, executor=SequentialExecutor() ) # with 100 retries the test will fail once in 10^31 asyncio.get_event_loop().run_until_complete(runner.task) assert runner.status() == "finished" From 230b356e3cacbbd0f8d53f377a1bec2c4b765648 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Thu, 16 Apr 2020 17:27:29 +0200 Subject: [PATCH 8/9] rename _IndexToPoint -> _CallFromSequence --- adaptive/learner/sequence_learner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/adaptive/learner/sequence_learner.py b/adaptive/learner/sequence_learner.py index 0069f53e2..99be75e79 100644 --- a/adaptive/learner/sequence_learner.py +++ b/adaptive/learner/sequence_learner.py @@ -5,7 +5,7 @@ from adaptive.learner.base_learner import BaseLearner -class _IndexToPoint: +class _CallFromSequence: """Call function with index of sequence.""" def __init__(self, function, sequence): @@ -45,7 +45,7 @@ class SequenceLearner(BaseLearner): def __init__(self, function, sequence): self._original_function = function - self.function = _IndexToPoint(function, sequence) + self.function = _CallFromSequence(function, sequence) self._to_do_indices = SortedSet({i for i, _ in enumerate(sequence)}) self._ntotal = len(sequence) self.sequence = copy(sequence) From 36cb23c91df8c05f033414c2d0de22f4cbc96e44 Mon Sep 17 00:00:00 2001 From: Bas Nijholt Date: Thu, 16 Apr 2020 17:50:41 +0200 Subject: [PATCH 9/9] simplify test --- adaptive/tests/test_sequence_learner.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/adaptive/tests/test_sequence_learner.py b/adaptive/tests/test_sequence_learner.py index 6cc06e963..68ca956ca 100644 --- a/adaptive/tests/test_sequence_learner.py +++ b/adaptive/tests/test_sequence_learner.py @@ -1,7 +1,5 @@ import asyncio -import numpy as np - from adaptive import Runner, SequenceLearner from adaptive.runner import SequentialExecutor @@ -14,15 +12,15 @@ def __call__(self, value): if self.failed: return value self.failed = True - raise Exception + raise RuntimeError def test_fail_with_sequence_of_unhashable(): # https://github.com/python-adaptive/adaptive/issues/265 - seq = [dict(x=x) for x in np.linspace(-1, 1, 101)] # unhashable + seq = [{1: 1}] # unhashable learner = SequenceLearner(FailOnce(), sequence=seq) runner = Runner( - learner, goal=SequenceLearner.done, retries=100, executor=SequentialExecutor() - ) # with 100 retries the test will fail once in 10^31 + learner, goal=SequenceLearner.done, retries=1, executor=SequentialExecutor() + ) asyncio.get_event_loop().run_until_complete(runner.task) assert runner.status() == "finished"