#9439: hyperbolic geometry
------------------------------------------------------------------------------+
Reporter: vdelecroix |
Owner: vdelecroix
Type: enhancement |
Status: new
Priority: major |
Milestone: sage-5.11
Component: geometry |
Resolution:
Keywords: hyperbolic geometry, Poincare disc, upper half plane, sd35 |
Work issues:
Report Upstream: N/A |
Reviewers: Johan Bosman
Authors: Vincent Delecroix, Martin Raum |
Merged in:
Dependencies: |
Stopgaps:
------------------------------------------------------------------------------+
Comment (by glaun):
Thanks for the feedback! I didn't know exactly what to do about tests in
what was intended to be an abstract class (hyperbeolic_object) that
shouldn't be instantiated. I see now that there are other abstract
classes in sage and they all have poper doctests, so I'll go ahead and add
those in. That should be very simple, as should the rest of the cleanup.
The other issues you mentioned are all things I thought might be problems,
so I've given them all a little bit of thought already
Replying to [comment:24 vdelecroix]:
> 4) the files should be properly included in the sage documentation
Okay, will do.
> Now, more serious issues
>
> 5) the objects from your module must be lazy imported and not imported
in the global namespace (in order to not slow down sage startup)
Okay, I will change this.
>
> 6) the method `show` should not return a graphics object but should
rather shows it! You could rename it `plot`.
I'm actually not quite sure what the difference is. Do you mean that show
should make a system call to the default viewer? Is it not sufficient to
call a method that makes that call for me? I've noticed in one other
module that plot() is implemented and show() is an alias for plot. Would
this be a workaround?
> 7) I agree that it is misleading but `CC` in Sage is not the set of
complex number! In particular the test `x in CC` does not answer to the
question "does my object `x` modelizes some complex number ?". Precisely
`CC` is the set of floating point complex number with 53 bits of
precision. But is it on purpose that you want `x in CC` as coordinates for
a point in the upper half plane ?
This is a hack. I wanted a function that returned True for things that can
be converted into floating point complex numbers and "2 + I in CC" returns
True even though "2 + I" is a symbolic expression. I didn't want to test
more explicitly for symbolic complex numbers because I don't know the
symbolic system well enough to be sure to catch all possible cases. And I
didn't want to have a test condition that relied on the symbolic apparatus
because via profiling I found many cases in which calling functions that
were in the symbolics library slowed everything to a crawl. So "x in CC"
quickly checks whether something is the right type of object to have
"imag()" called on it sensibly. I more than welcome suggestions on how to
solve this problem more elegantly without slowing down code. As an
example, the functions _clean_points and _shorten_symbolic are both very
ugly to my tastes, but I wrote each to address specific issues that arose
in profiling and their impact is significant. In a similar way, I find 'x
in CC' to be unfortunate but useful.
> 8) The string representation "Hyperbolic point whose representation in
the Upper Half Plane is +Infinity" is definitely too long! Why not "Point
in HH +Infinity".
The long strings were a request, and I agree they're much too long. I'll
go ahead and change them. I'd like them to contain information about the
model, though so that there is less confusion when performing conversion.
> And finally a design question (which perhaps should be thought of first)
>
> 9) I think that the fact of being conformal or bounded etc is not a
property of a HyperbolicObject but rather a property of the underlying
model. You decided to not design a class for each model, was it on
purpose? Such a class may contain all these property. As you see, in my
initial patch, it was possible to write
> {{{
> sage: HH
> Hyperbolic plane
> sage: HH(0)
> Boundary point 0
> }}}
> I would prefer to have a dedicated class for each model and be able to
construct point/geodesic/polygons from the class (via for example
``HH.point(data)``, ``HH.geodesic(data)``, etc). You may also move the
`random_element` methods and the various conversions between the models
into this parent class.
I think your way of doing it sounds much better than mine. I had actually
been planning on looking more closely at your structure and applying it to
mine as a next step. I'll look into this starting tomorrow.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/9439#comment:25>
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 unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/groups/opt_out.
