PolyhedronFace: Fix pickling test

[mkoeppe]

needed for #32013

We fix rich comparison of faces and pickled faces, so that as expected we have:

sage: P = polytopes.cube()
sage: f = P.faces(1)[2]
sage: f == loads(f.dumps())
True

Along the way we remove _test_pickling skips that are no longer necessary (because ppl polyhedra can be pickled).

Depends on #31959

CC: @kliem @tscrim

Component: geometry

Author: Jonathan Kliem

Branch/Commit: 2b9316b

Reviewer: Matthias Koeppe

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

[kliem]

comment:1

Can you please provide an example on where it fails? If you are going to fix it yourself then I guess you'll provide an example along with the fix.

[mkoeppe]

comment:2

The TestSuite fails -- currently all calls to TestSuite use skip='_test_pickling'.

I'm working on something else right now - so help with this ticket would be welcome!

[kliem]

comment:3

Well, the only problem is that we currently require face._polyhedron to be identical for two faces to be tested equal. Otherwise, everything is working, isn't it?

[mkoeppe]

comment:4

OK, perhaps it's too tricky to implement __richcmp__ for faces of polyhedra that are merely equal, not identical.

Although I suppose if the polyhedra are not the same, one could just delegate the comparison to as_polyhedron(...).

But I guess I can just skip the pickling test in #32013 to work around it.

[kliem]

comment:6

I didn't mean to imply that this does not need to be fixed. I just wanted to comment that the fixing needs to be done in richcmp and not pickling. Pickling works fine.

I guess instead of requiring the two polyhedra two be identical, we can just require the Vrepresentation and Hrepresentation to be the same:

    if self._polyhedron is not other._polyhedron:
-       raise NotImplementedError
+       if (self._polyhedron.Vrepresentation() != other._polyhedron.Vrepresentation() 
+               or self._polyhedron.Hrepresentation() != other._polyhedron.Hrepresentation()):
+           raise NotImplementedError

Actually we can be less strict and allow the Hrepresentation to differ. We could even allow the Vrepresentation to be a permutation of the other Vrepresentation, but this is definitely not needed to fix the pickling test for faces.

[kliem]

New commits:

981bb21	allow rich comparison of pickled faces
52a6f40	do not skip `test_pickling`, if it works

[kliem]

Author: Jonathan Kliem

[kliem]

Branch: public/32025

[kliem]

Commit: 52a6f40

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

686d0af	Polyhedron_base.product: Add doctest for alias 'cartesian_product'
2b1d108	Merge #31919
7323b10	ConvexSet_base._test_contains: Only test extension to AA for exact base rings
94e6858	RelativeInterior.ambient, ambient_vector_space, is_universe: New
0c9bc94	ConvexSet_base: Add default implementations of ambient, ambient_dim; add doctests
ce91e44	src/sage/geometry/relative_interior.py: Fix doctest output
f0e7c58	ambient_vector_space docstring: Fix bad blocks
200d967	ConvexSet_base.ambient doctest: Actually test the method
f02ca28	src/sage/geometry/polyhedron/face.py: Remove unused import
2b9316b	Merge #31959

[sagetrac-git]

Changed commit from 52a6f40 to 2b9316b

[mkoeppe]

comment:9

Merged to resolve merge conflict

[mkoeppe]

Dependencies: #31959

[mkoeppe]

Reviewer: Matthias Koeppe

[mkoeppe]

comment:10

Thanks for working on this! LGTM

[vbraun]

Changed branch from public/32025 to 2b9316b