Converter

[videlec]

This PR introduces a FlatsurfConverter class that could be used to pass around objects (such as singularities, points, segments, ...) between a fixed sage-flatsurf surface and a fixed pyflatsurf flat triangulation. Its constructor incorporates the code of the conversion functions from_pyflatsurf and to_pyflatsurf.

Partially solves #126. Fixes #193.

• [ ] NEWS
• [ ] choose a better name than FlatsurfConverter ?
• [ ] clarify the status of FlatsurConverter.flow_decomposition that was moved from GL2ROrbitClosure.decomposition
• [ ] implement saddle connection conversion
• [ ] implement point conversion

[videlec]

@saraedum Any idea about the failure in (sage,flipper,eantic,exactreal,pyflatsurf, binder, 3.7.12)?

[saraedum]

@saraedum Any idea about the failure in (sage,flipper,eantic,exactreal,pyflatsurf, binder, 3.7.12)?

Not sure what's the problem. Could there be some object involved that comes from a library that was built against an older version of sage? It's probably best to reproduce this locally by installing the same packages that were installed in the failing run.

[saraedum]

Could you explain what you are trying to achieve with these converters?

[videlec]

Could you explain what you are trying to achieve with these converters?

I tried to expand the PR description.

EDIT: the ultimate goal being to be able to play with intersections of a polygonal decomposition with the pull back of another one under a deformation.

[saraedum]

So that converter is more than just a namespace for conversion functions. It explicitly establishes the link between a surface on both sides? Is a main point to get caching of the surface on the other side?

[videlec]

The converter holds a fixed sage-flatsurf surface and a fixed pyflatsurf surface as attributes. It would allow to go back and forth between them. It is a kind of isomorphism.

About caching, it seemed natural to me to cache the FlatsurfConverter associated to a given sage-flatsurf surface. But maybe it is better otherwise.

[saraedum]

Caching sounds good to me in general. I am a bit concerned that the surfaces are not unique in this setup. I started to do something about this in #148 but it's complicated. So, the pyflatsurf surface associated to a sage-flatsurf surface is unique (because it is cached.) But when you convert back from the pyflatsurf surface you get different copies of the same translation surface.

So, why have the pair (pyflatsurf, sage-flatsurf) in an object if the link goes only in one direction anyway?

[videlec]

Indeed, this would not solve uniqueness. In particular constructing twice a FlatsurfConverter with the same underlying pyflatsurf flat triangulation would end up with two equal sage-flatsurf translation surfaces

sage: from pyflatsurf.vector import Vectors
sage: from pyflatsurf.factory import make_surface
sage: V2 = Vectors(QQ)
sage: vertices = [(1r, -3r, 2r, -1r, 6r, -5r), (-2r, 4r, -6r, 5r, -4r, 3r)]
sage: vectors = [V2((1, 1)), V2((-1, 0)), V2((0, -1)), V2((1, 1)), V2((-1, 0)), V2((0, -1))]
sage: T = make_surface(vertices, [v.vector for v in vectors])
sage: from flatsurf.geometry.pyflatsurf_conversion import FlatsurfConverter
sage: f1 = FlatsurfConverter(T)
sage: f2 = FlatsurfConverter(T)
sage: f1.sage_flatsurf_surface() == f2.sage_flatsurf_surface()
True
sage: f1.sage_flatsurf_surface() is f2.sage_flatsurf_surface()
False

The above can be solved by caching instances of FlatsurfConverter (a bit as UniqueRepresentation does it in sage).

[videlec]

So, why have the pair (pyflatsurf, sage-flatsurf) in an object if the link goes only in one direction anyway?

We need conversions in both directions. What do you propose without the creation of an object similar to FlatsurfConverter?

[saraedum]

So, why have the pair (pyflatsurf, sage-flatsurf) in an object if the link goes only in one direction anyway?

We need conversions in both directions. What do you propose without the creation of an object similar to FlatsurfConverter?

I am also not entirely sure about this. But if we say that a sage-flatsurf surface holds on to cached pyflatsurf surface and that sage-flatsurf surfaces are unique (at least in the sense that two equal pyflatsurf surfaces give the same sage-flatsurf surface) then you don't need that explicit link. Instead you could just convert a pyflatsurf point to a sage-flatsurf point by saying:

Convert point.surface() (which is the cached unique sage-flatsurf surface) and then convert point into a point in that sage-flatsurf surface.

[saraedum]

The point I am trying to make is probably, the converter object stores a pair (pyflatsurf, sage-flatsurf) thinking of both sides to be unique. But that's not true. Only the sage-flatsurf object can be unique. In the pyflatsurf world uniqueness is going to be close to impossible to achieve.

So, why not just have a lazy cached method/property _pyflatsurf on a sage-flatsurf surface and lookup the opposite conversion in a weak cache since that's needed anyway. In particular, consider the following. Take sage-flatsurf surface S and a corresponding pyflatsurf surface S' and store them in the converter associated to S. Now, given a point in a surface S'' that is equal but not identical to S', that converter is useless to convert that point since it's referencing the wrong pyflatsurf surface. So, all you need to convert a point is the point an the sage-flatsurf surface. So what use is the converter object. I wonder if this is just going to lead to complicated bugs since most of the time S' and S'' are identical but sometimes they are not.

Am I making sense?

[videlec]

The current converter can be made flexible enough to pull objects from any surface equal to FlatConverter.pyflatsurf_surface(). I agree that there is no need to construct and store the latter as an attribute in the FlatsurfConverter object. Still I would use a weak cache for FlatsurfConverter.pyflatsurf_surface() for efficiency.

However, for pulling objects from pyflatsurf, I don't see how that can work without a FlatsurfConveter. Uniqueness of translation surfaces is subtle. Because of labellings + choices of triangulation + isomorphisms, there are many non equal sage-flatsurf translation surfaces that give rise to the same pyflatsurf flat triangulation. In particular, it seems impossible to me to pull objects from a pyflatsurf surface without an explicit isomorphism such as FlatsurfConverter. Or maybe I misunderstood your proposal?

One thing that I want to do is to make the FlatsurfConverter constructor flexible enough so that one can specify completely the target pyflatsurf triangulation. In particular, there could be several FlatsurfConverter for the same source sage-flatsurf translation surface. Still, it would make sense to store a default one as a _pyflatsurf property.

[videlec]

I tried to incorporate your comments. Now the pyflatsurf flat triangulation is not exactly part of the FlatsurfConverter object but only a cached_method.

[videlec]

Interesting benchmark

sage: from flatsurf import translation_surfaces
sage: s = translation_surfaces.square_torus()
sage: %time len(s.saddle_connections(30*30))
CPU times: user 7.21 s, sys: 0 ns, total: 7.21 s
Wall time: 7.23 s
1704
sage: %time len([s._pyflatsurf.saddle_connection_from_pyflatsurf(sc, check=False) for sc in s._pyflatsurf.codomain().connections().bound(30r)])
CPU times: user 227 ms, sys: 3.93 ms, total: 231 ms
Wall time: 231 ms
1704

[videlec]

@saraedum Any idea to implement saddle connection conversion the other way around (sage-flatsurf -> pyflatsurf)?

[videlec]

@saraedum hmm... hit by https://trac.sagemath.org/ticket/29694 (only merged in some sage > 9.0). I am not sure how to fix it.

[slel]

Sage Trac ticket 29694 (conflict between lazy attribute and abstract method tester) was merged in Sage 9.2.beta7.

[saraedum]

Could we skip that test for older versions of SageMath?

We could also consider to bump the officially supported versions to be 9.2+.

[saraedum]

After all, even Debian stable has SageMath 9.2 now.

[slel]

Dropping support for Sage versions older than 2 years would seem fine to me. Some release dates:

• 2021-05-10 Sage 9.3
• 2020-10-24 Sage 9.2
• 2020-05-20 Sage 9.1

[saraedum]

I'll close this in favor of #211. We might some bits from here but I think I want to try that other idea first.