#10945: Fix lots of minor docs and redundancy for riemann.pyx
------------------------+---------------------------------------------------
Reporter: kcrisman | Owner: burcin
Type: defect | Status: new
Priority: minor | Milestone:
Component: calculus | Keywords: riemann map complex plot
Author: | Upstream: N/A
Reviewer: | Merged:
Work_issues: |
------------------------+---------------------------------------------------
Old description:
> 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'.
> * A lot of the code is redundant with `plot/complex_plot.pyx`. This
> should be factored out somehow, or imported, or whatever, unless it would
> lead to gross slowdowns.
> * Is there anything different about `complex_to_rgb` except that it's
> cdef'd (and the changes needed for that, including some efficiencies like
> the phase command)?
> * Similarly, the `ColorPlot` class is apparently identical to the
> `ComplexPlot` class, except that it's better documented, uses only the
> default interpolation, and won't take options.
> * Note that the documentation for `ColorPlot` only uses
> `complex_plot`!!! I view this as an error by mvngu and wdj in reviewing
> #6648, though that was a long review process so it's easy to see how it
> slipped through.
> * 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 a few analytic examples as mentioned at #10792.
> * 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).
New description:
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'.
* A lot of the code is redundant with `plot/complex_plot.pyx`. This
should be factored out somehow, or imported, or whatever, unless it would
lead to gross slowdowns.
* Is there anything different about `complex_to_rgb` except that it's
cdef'd (and the changes needed for that, including some efficiencies like
the phase command)?
* Similarly, the `ColorPlot` class is apparently identical to the
`ComplexPlot` class, except that it's better documented, uses only the
default interpolation, and won't take options.
* Note that the documentation for `ColorPlot` only uses
`complex_plot`!!! I view this as an error by mvngu and wdj in reviewing
#6648, though that was a long review process so it's easy to see how it
slipped through.
* 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).
--
Comment(by kcrisman):
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.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10945#comment:5>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
