Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix lots of minor docs and redundancy for riemann.pyx #10945

Closed
kcrisman opened this issue Mar 16, 2011 · 11 comments
Closed

Fix lots of minor docs and redundancy for riemann.pyx #10945

kcrisman opened this issue Mar 16, 2011 · 11 comments

Comments

@kcrisman
Copy link
Member

The Riemann mapping stuff is a great addition - excellent work!

But there are lots of minor problems that should be fixed. Here are some. Closing this ticket wouldn't require fixing them all, but if not, then explanations should be provided here and followup tickets (if needed) opened.

  • Awkward phrasing:
    Note that all the methods are numeric rather than analytic, for unusual
    regions or insufficient collocation points may give very inaccurate
    results.

This is hard to follow, though I see what it says.

  • There is also a 'reimann' several times, and a 'correspondance'.
  • This also seems to have a transposition in how it's put in. Presumably the redefinition of I is supposed to demonstrate it works with lots of different complex types. I suggest the following.
        Can work for different types of complex numbers::

            sage: m = Riemann_Map([lambda t: e^(I*t) - 0.5*e^(-I*t)], [lambda t: I*e^(I*t) + 0.5*I*e^(-I*t)], 0)  # long time (4 sec)
            sage: m.riemann_map(0.25 + sqrt(-0.5))  # long time
            (0.137514137885...+0.876696023004...j)
+            sage: I = CDF.gen()  # long time
            sage: m.riemann_map(1.3*I)  # long time
            (-1.561029396...+0.989694535737...j)
-            sage: I = CDF.gen()  # long time
            sage: m.riemann_map(0.4)  # long time
  • Add any information at all about theoretical error bounds, if known. Since not everyone will be able to just look up that paper referenced, it would be helpful to have at least order of magnitude ideas (e.g., if N=2000 on a map from the unit circle to itself, we expect errors no greater than epsilon=blah).
  • From speed up the riemann mapping functionality #8867: "It now properly avoids failing with lambda functions, although it doesn't work optimally for them. I'll add some notes on that in Fix lots of minor docs and redundancy for riemann.pyx #10945."

CC: @sagetrac-evanandel @jasongrout @wdjoyner @sagetrac-mvngu

Component: calculus

Keywords: riemann map complex plot

Reviewer: Karl-Dieter Crisman

Issue created by migration from https://trac.sagemath.org/ticket/10945

@kcrisman

This comment has been minimized.

@kcrisman

This comment has been minimized.

@kcrisman

This comment has been minimized.

@sagetrac-evanandel
Copy link
Mannequin

sagetrac-evanandel mannequin commented Mar 17, 2011

comment:4

Regarding the documentation, thanks for pointing that out. I'll get on it. Since I'm working on this as a senior project, I appreciate any documentation/setup feedback since I can stick that under the category of "usability testing".

I've separated the analytic tests under #10857. I'll get to work on that soon.

With regards to the plotting, I plan to do some reworking of that whole area. I'm going to add options for parallelization of the grid computation. I also intend to make the display function modular so that different "color schemes" can be used. For example I want the interested user to be able to specify the bright/dark ranges. I also want to add an error plot that shows estimates of the numerical error.

Now the reason I made my "clone" of complex_plot in the first place was to minimize the function call overhead by keeping it all in-house where I can take advantage of the cdef. However some of the work I want to do could be pretty general, thus it might be worth it to put it somewhere in the plot package.

Thoughts?

@kcrisman

This comment has been minimized.

@kcrisman
Copy link
Member Author

comment:5

Description updated.

I'm not the Cython expert. But if you cc: Jason and Robert Bradshaw, you should get some good help :) I don't see why one couldn't cimport the complex plot, after all. Maybe not? Modularity is good, I don't think people will be plotting so very many of these as to worry about the millisecond it takes to import.

@sagetrac-evanandel
Copy link
Mannequin

sagetrac-evanandel mannequin commented Mar 22, 2011

comment:6

Correction, analytic tests are under #10957

@kcrisman

This comment has been minimized.

@kcrisman
Copy link
Member Author

comment:7

#11028 is taking care of the modularity and ColorPlot issues, I think.

@kcrisman
Copy link
Member Author

kcrisman commented Jun 9, 2011

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member Author

kcrisman commented Jun 9, 2011

comment:8

Okay, I've checked everything, and this is superseded by #11273 and #11028. Please close this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants