I propose to:

• Repurpose SequenceLearner with unhashable entries in sequence break Runner #265 to an issue that runner breaks on unhashable inputs: that's the real problem.
• Make this PR dedicated to a simplification of the sequence learner (which happens to close the current SequenceLearner with unhashable entries in sequence break Runner #265 a side effect).
• Also throw away the unnecessary __getstate__/__setstate__ logic from _IndexToPoint.

@basnijholt what do you think?

Codecov Report

Merging #266 into master will increase coverage by 0.16%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #266      +/-   ##
==========================================
+ Coverage   79.52%   79.68%   +0.16%     
==========================================
  Files          32       33       +1     
  Lines        4424     4416       -8     
  Branches      815      814       -1     
==========================================
+ Hits         3518     3519       +1     
+ Misses        779      768      -11     
- Partials      127      129       +2     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d9397d...230b356. Read the comment docs.

@akhmerov, I've implemented the changes you suggested and opened #267.

Thanks for your review! 🙌

Sorry, I'm having second thoughts. Passing the complete sequence of arguments to the function means we're distributing it across all workers. Is that definitely OK? It might be, but I think we should consider this carefully.

Passing the complete sequence of arguments to the function means we're distributing it across all workers. Is that definitely OK?

Ultimately I think this would only be a problem in extremely exotic cases, and the simplification of the codebase justifies this downside.

I actually lean towards not accepting the overhead.

From learning the following function on a ProcessPoolExecutor:

class FailOnce:
    def __init__(self):
        self.n = 0

    def __call__(self, value):
        self.n += 1
        return self.n

Which will always return 1, I learn that the sequence will be serialized and send each point!

As an alternative solution, I have opened https://github.com/python-adaptive/adaptive/pull/268/files. Which appeared to be way less work than I thought initially.

Superseded by #268.

I'm actually not sure whether creating multiple copies of a sequence is an important limitation and I am wondering whether we should merge this anyway. I still believe this implementation of a sequence learner is simpler, and therefore deserves consideration separate of its relation to #265.

@jbweston, what do you think?

@akhmerov, I've benchmarked this and the overhead is completely unacceptable.

For example, at this exact moment I am doing simulations with SequenceLearners of ~1e6 items with 15 parameters.

import pickle
import numpy as np
sequence = list(np.random.rand(1_000_000, 15))
%timeit pickle.dumps(sequence)

4.92 s ± 44.2 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
An that happens on each function evaluation. And this does not even include unserializing and sending it over the network.

OK, fair enough.