Opened 8 years ago
Closed 8 years ago
#10945 closed defect (duplicate)
Fix lots of minor docs and redundancy for riemann.pyx
Reported by: | kcrisman | Owned by: | burcin |
---|---|---|---|
Priority: | minor | Milestone: | sage-duplicate/invalid/wontfix |
Component: | calculus | Keywords: | riemann map complex plot |
Cc: | evanandel, jason, wdj, mvngu | Merged in: | |
Authors: | Reviewers: | Karl-Dieter Crisman | |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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 #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 #10945."
Change History (12)
comment:1 Changed 8 years ago by
- Description modified (diff)
comment:2 Changed 8 years ago by
- Description modified (diff)
comment:3 Changed 8 years ago by
- Description modified (diff)
comment:4 Changed 8 years ago by
comment:5 follow-up: ↓ 6 Changed 8 years ago by
- Description modified (diff)
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.
comment:6 in reply to: ↑ 5 Changed 8 years ago by
Correction, analytic tests are under #10957
comment:7 Changed 8 years ago by
- Description modified (diff)
#11028 is taking care of the modularity and ColorPlot issues, I think.
comment:8 Changed 8 years ago by
- Reviewers set to Karl-Dieter Crisman
- Status changed from new to needs_review
comment:9 Changed 8 years ago by
- Status changed from needs_review to positive_review
comment:10 Changed 8 years ago by
- Milestone set to sage-4.7.1
comment:11 Changed 8 years ago by
- Milestone changed from sage-4.7.1 to sage-duplicate/invalid/wontfix
comment:12 Changed 8 years ago by
- Resolution set to duplicate
- Status changed from positive_review to closed
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?