Opened 9 months ago
Closed 8 months ago
#23257 closed enhancement (fixed)
Plotting the Mandelbrot set in Sage
Reported by:  bbarros  Owned by:  

Priority:  minor  Milestone:  sage8.1 
Component:  dynamics  Keywords:  gsoc2017, complexdynamics 
Cc:  atowsley, kcrisman  Merged in:  
Authors:  Ben Barros  Reviewers:  Ben Hutz 
Report Upstream:  N/A  Work issues:  
Branch:  f52fb3f (Commits)  Commit:  f52fb3fc9b32f6c24bdf3f308740603933d44600 
Dependencies:  Stopgaps: 
Description (last modified by )
This ticket is the first in a series of tickets that will be opened this summer in an effort to improve the functionality for complex dynamics in Sage. I have added complex_dynamics folder to the dynamics folder that introduces the function mandelbrot_plot() which allows users to produce an interactive plot of the Mandelbrot set. For more information on my Google Summer of Code Project you can visit the following link: https://benbarros.wordpress.com/
Change History (43)
comment:1 Changed 9 months ago by
 Branch set to u/bbarros/23257
comment:2 Changed 9 months ago by
 Commit set to 023321fa6b77e9f749c0fb020bd85c41d9ca8858
 Status changed from new to needs_review
comment:3 Changed 9 months ago by
 Cc atowsley added
 Keywords gsoc2017 complexdynamics added; GSoC complex dynamics removed
 Priority changed from major to minor
 Reviewers set to Ben Hutz
 Status changed from needs_review to needs_work
First, some general things:
 don't include the helper file in the documentation (rst)
 also do not add the pyx extension since the helper functions aren't exposed to the user
 I'd add to the ticket description that this is the first of a series improving the complex dynamics functionality.
for the .py file
 OUTPUT:  isn't this a .bmp or an interact?
 kwds go under an INPUT: section
 add some readability spacing, such as after , for arguments
kwds.pop("x_center", 1.0)
 I don't understand the section 'NOTEBOOK EXAMPLES:'. Seems like this is all selfexplanatory from the kwds description. The actual function call examples are needed, but they should be in an EXAMPLES: section
 I would add some to more the general function description:
 what is the mandelbrot set
 how are you computing it (perhaps a very basic ALGORITHM: section)
for the .pyx file
 I don't think you need the initial underscore (that marks it as a private function that won't appear in tab completion) since it is not exposed to the user anyway.
 need one line description of function
 why not a cdef?
 OUTPUT  isn't this a .bmp
 The examples fail because you need to import the function first
sage: from sage.dynamics.complex_dynamics.mandel_julia_helper import _fast_mandel_plot
 bug: negative image_width flips the image instead of gives an error
a couple functionality thoughts:
for interact:
 I'd let the user adjust pretty much anything
 I still don't really like the width slider. How about a input box?
for colors: the contour levels are not great by default. I was playing around with it and the best I could come up with using basically the same system was as follows
have two parameters
 number of iterations between levels (default to 1)
 number of colors to use
you'll need to do better with your nested ifs. For example
cdef int level if iteration != max_iteration: level = iteration/level_sep if level < color_num: pixel[row,col] = color_list[level] else: pixel[row,col] = color_list[1]
This gave me pretty good control over the contours and I got a nice greyscale image with base_color [50,50,50] with level_sep=1 and 20 colors gradations. This has the advantage of disengaging it from the max_iterations. So you can increase the detail without messing up the contours.
I'll leave the default color choice to you, but at least consider gray scale since it should allow a 'white' external ray to show up well.
comment:4 Changed 9 months ago by
 Description modified (diff)
comment:5 Changed 9 months ago by
 Description modified (diff)
comment:6 Changed 9 months ago by
 Commit changed from 023321fa6b77e9f749c0fb020bd85c41d9ca8858 to dbcd279521f5e5a76258981f4607bd4b4ad4ec90
Branch pushed to git repo; I updated commit sha1. New commits:
dbcd279  Updated and debugged complex_dynamics files

comment:7 Changed 9 months ago by
 Status changed from needs_work to needs_review
I pushed an updated branch with the changes made. I did not change the function in the .pyx file to a cdef because whenever I try to import it as a cdef (using or cimport or just import) I am unable to import it. If you have any ideas of how to get that to work I would love to hear them. Please let me know what else I need to fix.
comment:8 Changed 9 months ago by
 Commit changed from dbcd279521f5e5a76258981f4607bd4b4ad4ec90 to bcb3792080656f66a79b3c88b8b8680380db185c
Branch pushed to git repo; I updated commit sha1. New commits:
bcb3792  23257: Fixed syntax and doctests

comment:9 Changed 9 months ago by
 Status changed from needs_review to needs_work
I think the functionality works well now. I have a few code nitpicks and one suggestion for speedup.
You also need to resolve the current merge conflict
cython file:
import statements to top
line 89 too long
line 98: don't over do the spacing: 2*new_x*new_y + y_coor
add some comments describing what each code block is doing. for example, why are you reflecting about the xaxis. Are you trying to use that it is symmetrical? In that case you should be assigning two pixel values for each iteration and only looping through the top half of the pixels, but only when the real axis is contain in the image window.
speed ups: the quantities
image_width * (rowpixel_count / 2) / pixel_count image_width * (colpixel_count / 2) / pixel_count
are better off precomputed as step values
python file
reference should go in the main reference file: src/doc/en/reference/references/index.rst
then you can reference it here as [Devaney]_
you also have several lines longer than the 80char standard. They should be wrapped.
several examples are missing the 'sage:' portion
imports to top of file
comment:10 Changed 9 months ago by
 Commit changed from bcb3792080656f66a79b3c88b8b8680380db185c to 6a9f9531f02139f0d2d94c68dcd495902c8ac4e6
Branch pushed to git repo; I updated commit sha1. New commits:
6a9f953  23257: Fixed documentation and syntax

comment:11 Changed 9 months ago by
 Cc kcrisman added
comment:12 Changed 9 months ago by
If you are doing this sort of thing, maybe eventually it will supersede e.g. #8423, #1004, #11837 ? See also this old sagedevel thread.
comment:13 followup: ↓ 15 Changed 9 months ago by
Various comments I hope are useful:
 This branch doesn't apply to latest develop, apparently?
 Separately, it would be useful to know whether the interactive functionality works anywhere than in the SageNB notebook. Jupyter? SMC sagews? Sage Cell Server?
 Also, one could probably use the newish Sphinx plot directive thingamabob to put a few in the documentation...
 Does this plot Julia sets, as the file seems to indicate? What about plot for anything other than the square+add map? (E.g. z^{3+c.) }
 How long does testing these take? You may need to mark some tests
# long time
.
I understand some of this may be answered by your work later in the GSOC summer but wanted to try to get that all out there. Good luck  as you can tell from some of my references in comment:12, this has been missing in Sage for a LONG time.
comment:14 Changed 9 months ago by
 Commit changed from 6a9f9531f02139f0d2d94c68dcd495902c8ac4e6 to 1afa33239d14c19261da3f710a7849532019f16b
comment:15 in reply to: ↑ 13 Changed 9 months ago by
Thanks for the feedback! The plotting of the Julia set is what we plan to work on next after finishing up everything with the Mandelbrot set so we thought we should name the functions accordingly. I have not tried to use the interact functionality in anywhere besides the Sage notebook so I will be sure to try those out and make a note of it in the documentation. I am also not familiar with Sphinx so I will have to take a look at that. As far as a more general version of the function, we plan to get to that after we tackle Julia sets.
Thanks again,
Ben Barros
Replying to kcrisman:
Various comments I hope are useful:
 This branch doesn't apply to latest develop, apparently?
 Separately, it would be useful to know whether the interactive functionality works anywhere than in the SageNB notebook. Jupyter? SMC sagews? Sage Cell Server?
 Also, one could probably use the newish Sphinx plot directive thingamabob to put a few in the documentation...
 Does this plot Julia sets, as the file seems to indicate? What about plot for anything other than the square+add map? (E.g. z^{3+c.) }
 How long does testing these take? You may need to mark some tests
# long time
.I understand some of this may be answered by your work later in the GSOC summer but wanted to try to get that all out there. Good luck  as you can tell from some of my references in comment:12, this has been missing in Sage for a LONG time.
comment:16 Changed 9 months ago by
I added a TESTS block in the documentation of mandelbrot_plot and changed all of the examples to have the flag # not tested. I don't quite like the look of this in the documentation but it was the only I way I could think of for now. Here is an example of what I am talking about:
sage: mandelbrot_plot() # not tested sage: mandelbrot_plot(pixel_count=1000) # not tested sage: mandelbrot_plot(base_color=[70, 40, 240]) # not tested sage: mandelbrot_plot(x_center=0.75, y_center=0.25, image_width=1/2, number_of_colors=75) # not tested "To use the function outside of the notebook, you must set interact to False" sage: mandelbrot_plot(interact=False) # not tested Launched png viewer for 500x500px 24bit RGB image sage: mandelbrot_plot(interact=False, x_center=1.11, y_center=0.2283, image_width=1/128, # not tested ....: max_iteration=2000, number_of_colors=500, base_color=[40, 100, 100]) Launched png viewer for 500x500px 24bit RGB image
comment:17 Changed 9 months ago by
 Status changed from needs_work to needs_review
comment:18 followup: ↓ 19 Changed 9 months ago by
An easy fix: lighter not darker for more iterations
I understand preventing the huge html ones from running in the EXAMPLES so that the user isn't bombarded. It is good that they are there so the user can see how to call the function with various parameters. But why are the noninteract ones blocked as well? It seems that you just add them later in TESTS section. I'd just put them in the example section and let them run.
Do you still need to add the 'Extension' for importing the .pyx? Since those are now private functions, I didn't think you needed that anymore.
I can't say I like the interact html tests. Maybe post a question to sagedevel about how to test an interact with a doctest, or even if you should be trying to.
comment:19 in reply to: ↑ 18 Changed 9 months ago by
I can't say I like the interact html tests. Maybe post a question to sagedevel about how to test an interact with a doctest, or even if you should be trying to.
Don't bother trying to actually test them. BUT you should definitely still include this as documentation (not TESTS
) so that people know what the heck to do; the point is to get it used.
I highly recommend creating a CoCalc? account and trying this with the Jupyter (or even locally) and .sagews format. You'd need some kind of internet access or special permission from William to try out your branch, but that should be doable. I don't see any reason it shouldn't work though if you are using the same widgets.
comment:20 Changed 9 months ago by
 Status changed from needs_review to needs_work
 For example about how to include and test interacts, see
src/sage/interacts/library.py
.
 There is a specific widget for colors, use that instead of an input box.
 Don't
show()
the plot, but return the plot.
 In the Cython code: why use
float
which has limited precision? I would guess that things get more interesting withdouble
precision, especially for deeper zooms. Second, it's less important, but I would uselong
instead ofint
. It would be unlikely that somebody would use more than 2^{31} pixels or iterations, but at least we should not a priori exclude it either.
 In the Cython code: use
sig_check
to make the code interruptible, see http://cysignals.readthedocs.io/en/latest/interrupt.html
 In the Cython code: better add typing for every variable like
x_step
.
 To be compatible with Python 3: add
from __future__ import absolute_import, division
at the top of your files and consider whether you want floor division (use//
in that case) or true division.
comment:21 Changed 9 months ago by
 Obviously, do test
mandelbrot_plot()
(I guess the reason that you don't is number 3 from my last comment).
 The formatting of
TESTS
is wrong, it should beTESTS::
with indentation.
comment:22 Changed 9 months ago by
Okay, this is pretty impressive. But still some stuff for sure needs to be done. Jeroen has some very good ideas for Cython.
I'm not sure whether the default should be interactive or not. Usually one would make the static one the default.
Second, it's not clear to me that the only way one wants to interact with this is via typing in things. Sliders make much more sense for at least generic zooming exploration. I don't know whether this should be an option or show people how to see them ...
comment:23 Changed 9 months ago by
I just saw that there is an existing Mandelbrot interact implementation interacts.fractals.mandelbrot()
in src/sage/interacts/library.py
. You should at least try that one and maybe see if you can reuse something or replace that one with your version...
comment:24 Changed 9 months ago by
Ha, I totally forgot about that one! Indeed,
from .library import mandelbrot, julia, cellular_automaton
Hopefully the work you did here is good background for something far more general ...
comment:25 Changed 9 months ago by
The plan is to get the Mandelbrot set plotted and then move on to more general functionality such as plotting external rays on the Mandelbrot set, plotting of Julia sets, and eventually plotting the Mandelbrot set for more general functions.
comment:26 Changed 9 months ago by
 Commit changed from 1afa33239d14c19261da3f710a7849532019f16b to 9b6738b06bcfa199a4f7148a3cde5218fdb5f012
Branch pushed to git repo; I updated commit sha1. New commits:
9b6738b  23257: Cython, Python 3 improvements, Color widget added

comment:27 Changed 9 months ago by
I cleaned up the documentation, added sig_check() to make the function interruptable, Python 3 division, changed the floats to doubles, and ints to longs. I have been playing around with sliders for a while and it isn't very easy to get the precision required when zooming in. They may be convenient for navigating around the image but they are usually too sensitive when you zoom in far enough. I plan to replace the code in the interacts library eventually. However, the function there is slightly more general than mine right now (it plots the set for different exponents of z) so I will leave it until I can replace it with a more general version.
comment:28 Changed 9 months ago by
 Status changed from needs_work to needs_review
comment:29 Changed 9 months ago by
I have been playing around with sliders for a while and it isn't very easy to get the precision required when zooming in. They may be convenient for navigating around the image but they are usually too sensitive when you zoom in far enough.
Oh, totally. However, it's also very hard to navigate using just the typedin numbers. Maybe there can be some combination of that ... I wonder. Anyway, something to think about.
I plan to replace the code in the interacts library eventually.
I assume yours ends up being faster and/or more accurate? That will be welcome.
comment:30 followup: ↓ 32 Changed 9 months ago by
Why is it called fast_mandel_plot
and not fast_mandelbrot_plot
?
comment:31 Changed 9 months ago by
 Commit changed from 9b6738b06bcfa199a4f7148a3cde5218fdb5f012 to a1d02e4c35099cfe7bc344fd8647b38b526eacbb
Branch pushed to git repo; I updated commit sha1. New commits:
a1d02e4  23257: Changed fast_mandel_plot to fast_mandelbrot_plot

comment:32 in reply to: ↑ 30 Changed 9 months ago by
Replying to vdelecroix:
Why is it called
fast_mandel_plot
and notfast_mandelbrot_plot
?
Good question. I never really thought about that. I could change that pretty easily though.
New commits:
a1d02e4  23257: Changed fast_mandel_plot to fast_mandelbrot_plot

comment:33 Changed 9 months ago by
 Status changed from needs_review to needs_work
This is looking good, I just have a couple minor nitpicks.
 in the INPUT
 you have f(z) in max_iteration instead of Q_c(z) (both files)
 interact to false is not just for outside the notebook(in the .py file)
 I thought you were going to change the default for interact to False
 line 78 of .pyx : initialize misspelled
 It took me a little bit to figure out why your y_step uses y_center. Then I realized that it isn't really the y_step, that is scale_factor. Rather x_step, y_step represent the upper left hand corner of the image. And scale_factor really is the step_size. Then you are using the xaxis symmetry of the image. Any reason, you are not just using the correct y coordinate? Why don't you just use
y_step = y_center + image_width/2 y_coor = y_step  col*scale_factor
 I also suggest renaming those variables to match their purpose and making a comment that you are starting at the upper left hand corner of the image.
 line 102: sig_check not needed (you already check in the inside loop)
comment:34 Changed 9 months ago by
 Commit changed from a1d02e4c35099cfe7bc344fd8647b38b526eacbb to 8aae69cc30ad0e566e0e1fe6f7646822b91df50c
Branch pushed to git repo; I updated commit sha1. New commits:
8aae69c  23257: Changed default for interact to False

comment:35 Changed 9 months ago by
 Status changed from needs_work to needs_review
I changed the default for the interact to False and consequently changed the examples a little bit. I also renamed and cleaned up the variables dealing with the coordinates. Please let me know if there's anything else that catches your attention.
comment:36 Changed 9 months ago by
 Status changed from needs_review to needs_work
Went to build the docs from scratch and there is an error:
[dochtml] [dynamics ] /home/ben/sage/sagetest/src/doc/en/reference/dynamics/complex_dynamics.rst:: WARNING: document isn't included in any toctree [dochtml] Error building the documentation.
While we're renaming variables. Aren't your 'row' and 'col' variable as reversed.
Everything else looks fine
comment:37 Changed 9 months ago by
To be fair, that could be an unrelated thing due to switching between branches; try make docclean
and then make the doc again? See #22588.
comment:38 Changed 9 months ago by
It was a clean doc build.
comment:39 Changed 9 months ago by
 Commit changed from 8aae69cc30ad0e566e0e1fe6f7646822b91df50c to f52fb3fc9b32f6c24bdf3f308740603933d44600
Branch pushed to git repo; I updated commit sha1. New commits:
f52fb3f  23257: Added complex_dynamics to dynamics/index.rst

comment:40 Changed 9 months ago by
 Status changed from needs_work to needs_review
comment:41 Changed 8 months ago by
 Milestone changed from sage8.0 to sage8.1
 Status changed from needs_review to positive_review
Since 8.0.rc0 I out, I assume this gets pushed to 8.1 now.
comment:42 Changed 8 months ago by
 Component changed from fractals to dynamics
comment:43 Changed 8 months ago by
 Branch changed from u/bbarros/23257 to f52fb3fc9b32f6c24bdf3f308740603933d44600
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Added complex_dynamics folder for plotting Mandelbrot set