esciencecenter-digital-skills / parallel-python-workshop

Environment for the Parallel Python workshop
Apache License 2.0
6 stars 7 forks source link

Mandelbrot official solution not working for me either #11

Open erikkemperman opened 2 years ago

erikkemperman commented 2 years ago

As promised, I just tried the mandelbrot version from the solution branch, and unfortunately I run into the same (or very similar) problem with numba vectorize:

UFuncTypeError: ufunc 'mandelbrot_vec' did not contain a loop with signature matching types (<class 'numpy.dtype[complex128]'>, <class 'numpy.dtype[int64]'>) -> <class 'numpy.dtype[int64]'>

I'm on a recent macbook pro with M1 chip, and conda install gave me numba.__version__ == '0.53.1'.

Minor (unrelated) detail, in the 'Hint' section where the innermost loop is factored out as its own function, you're using the constant MAX_ITER rather than the function arg max_iter:

https://github.com/esciencecenter-digital-skills/parallel-python-workshop/blame/main/mandelbrot/Mandelbrot.ipynb#L224

And the same is true in the solution:

https://github.com/esciencecenter-digital-skills/parallel-python-workshop/blame/exercise-solutions/mandelbrot/Mandelbrot_solution.ipynb#L290

As a further minor suggestion, it might be worthwhile to make a separate function def mandelbrot_render(niters) with the plotting logic so we can easily check if our changes don't mess up the results. In that case, though, it's important to always use niters = compute_mandelbrot(...) everywhere, and not just the naked compute_mandelbrot().

Anyway, thanks for a great workshop, hope this helps a bit.

erikkemperman commented 2 years ago

Oh, forgot to mention: perhaps I hugely misunderstood things... But my intuition was that dask arrays would be a more natural fit here than the dask bags proposed in the solution -- since all the data is numerical?

erikkemperman commented 2 years ago

Sorry, one more thing -- of course I couldn't leave it alone :-)

But in the interest of constructive feedback I thought I should let you know that the official solution does work on my mac M1 (if that was even the issue) with numba >= 0.54, without any other changes. Actually 0.55 was released on PyPI yesterday but doesn't seem to be available in conda yet. That also requires some newer versions of numpy and its dependencies -- overall I might suggest that an update to environment.yml might benefit future editions of this excellent workshop.

Also, I just notice that dask arrays are also mentioned in the official solution here, just not where I would expect them (immediately after dask bags, or maybe even instead of it). So please disregard my previous comment.