Opened 2 months ago

Closed 8 weeks ago

#23425 closed enhancement (fixed)

Plotting External Rays on the Mandelbrot Set

Reported by: bbarros Owned by:
Priority: minor Milestone: sage-8.1
Component: dynamics Keywords: GSoC2017, ComplexDynamics
Cc: bhutz, atowsley Merged in:
Authors: Ben Barros Reviewers: Ben Hutz
Report Upstream: N/A Work issues:
Branch: 7a7d890 (Commits) Commit: 7a7d8908eceb8e38116b76b86cf68e79c99f4ff4
Dependencies: #23257 Stopgaps:

Description

This is the second ticket for my Google Summer of Code project (the first was #23257: Plotting the Mandelbrot set). I have added the external_ray function to the Complex Dynamics folder. This function allows the user to pass in an angle or list of angles and returns the plot of the external rays for the given angles on 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 (11)

comment:1 Changed 2 months ago by bbarros

  • Branch set to u/bbarros/23425

comment:2 Changed 2 months ago by bbarros

  • Commit set to 62802172b0e67bcd53fce6d781bdf13914e6f46d
  • Status changed from new to needs_review

Last 10 new commits:

6a9f95323257: Fixed documentation and syntax
f74fac8Merge branch 'master' into 23257
e1a379123257: Changed to lazy_import in complex_dynamics/all.py
1afa33223257: Fixed merge conflict and doctests
9b6738b23257: Cython, Python 3 improvements, Color widget added
a1d02e423257: Changed fast_mandel_plot to fast_mandelbrot_plot
8aae69c23257: Changed default for interact to False
f52fb3f23257: Added complex_dynamics to dynamics/index.rst
607cc2dMerge branch 'develop' of git://trac.sagemath.org/sage into ExternalRays
628021723425: Added external_ray function, helper functions.

comment:3 Changed 2 months ago by bhutz

  • Reviewers set to Ben Hutz
  • Status changed from needs_review to needs_work

I haven't had a chance to do much testing yet. So far it appears to work well in the notebook, but I need to do a thorough code review. Here is at least one error:

sage: external_ray(3/7,D=70)
ValueError: cannot convert float NaN to integer

if you zoom in, it looks like something is going wrong with the later Newton's methods

sage: external_ray(3/7,D=60, image_width=0.1, x_center=-1.75)

here are a few minor things I picked out:

  • need to input check 0<= ray <=1
  • need to wrap the long lines (90 characters is the typical limit)
  • add a ray color example
  • change the first long example to a decimal
  • in warning:
    • typo 'specify the which'
  • you shouldn't need to keywords for the mandelbrot set here.

It would be cleaner to default image to None. Then after you .pop image you check if 'is None' and if true mandelbrot_plot. If you use .get() instead of .pop() you can pass kwds directly to mandelbrot_plot.

  • you can just say, theta = list(theta) instead of
        if type(theta) != list:
            theta = [theta]
    
  • put something in the doc about: if the ray doesn't reach the boundary increase D, if the ray looks jagged increase S. Has a speed penalty

comment:4 Changed 2 months ago by bhutz

  • Component changed from fractals to dynamics

comment:5 Changed 2 months ago by git

  • Commit changed from 62802172b0e67bcd53fce6d781bdf13914e6f46d to 9aabbed7018fb0f5d10e9c512040085a5655a9ec

Branch pushed to git repo; I updated commit sha1. New commits:

9aabbed23425: Bug fixes, edited examples

comment:6 Changed 2 months ago by bbarros

  • Status changed from needs_work to needs_review

The following code didn't work:

theta = list(theta)

Since it returns the following error:

TypeError: 'sage.rings.integer.Integer' object is not iterable

comment:7 Changed 2 months ago by bhutz

  • Status changed from needs_review to needs_work

Just one very minor comment

In the description of the input R there is an extra word or something. Also, I'd add the comment similar to the other parameters that if R is too small Newton's method may not converge to the correct ray.

comment:8 Changed 2 months ago by git

  • Commit changed from 9aabbed7018fb0f5d10e9c512040085a5655a9ec to 7a7d8908eceb8e38116b76b86cf68e79c99f4ff4

Branch pushed to git repo; I updated commit sha1. New commits:

7a7d89023425: Updated documentation

comment:9 Changed 2 months ago by bbarros

  • Status changed from needs_work to needs_review

comment:10 Changed 2 months ago by bhutz

  • Status changed from needs_review to positive_review

comment:11 Changed 8 weeks ago by vbraun

  • Branch changed from u/bbarros/23425 to 7a7d8908eceb8e38116b76b86cf68e79c99f4ff4
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.