Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polyhedron_normaliz.save #26363

Closed
mkoeppe opened this issue Sep 29, 2018 · 26 comments
Closed

Polyhedron_normaliz.save #26363

mkoeppe opened this issue Sep 29, 2018 · 26 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Sep 29, 2018

sage: P = polytopes.dodecahedron(backend='normaliz')
sage: P
A 3-dimensional polyhedron in (Number Field in sqrt5 with defining polynomial x^2 - 5)^3 defined as the convex hull of 20 vertices
sage: P.save('dodecahedron.sobj')
TypeError: can't pickle PyCapsule objects

We fix this by removing the cone with __getstate__ on pickling.

On unpickling we use __setstate__ and _cone_from_Vrepresentation_and_Hrepresentation from #28639 to restore the cone.

Special care has to be taken in the following cases:

  • no inequalities (the cone can only be initialized from Vrep),
  • the empty polyhedron (cone is None in this case).

As the lines are recomputed, there is no guarantee that they appear in the same order in the normaliz cone. However, normaliz sorts the given lines anyway:

sage: P = Polyhedron(lines=[[1,0], [0,1]], backend='normaliz').lines()
(A line in the direction (1, 0), A line in the direction (0, 1))
sage: P = Polyhedron(lines=[[0,1], [1,0]], backend='normaliz').lines()
(A line in the direction (1, 0), A line in the direction (0, 1))
sage: P = Polyhedron(lines=[[1,1], [1,0]], backend='normaliz').lines()
(A line in the direction (1, 0), A line in the direction (0, 1))

Also, even if _normaliz_cone has the lines somewhat shuffled, this shouldn't be noticable as computations are invariant on which line we choose.

Depends on #28639

CC: @jplab @w-bruns @tscrim @kliem

Component: geometry

Author: Jonathan Kliem

Branch: 57beae3

Reviewer: Travis Scrimshaw

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

@mkoeppe mkoeppe added this to the sage-8.4 milestone Sep 29, 2018
@mkoeppe

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Jul 5, 2019

comment:2

One could define in Polyhedron_normaliz:

def __getstate__(self):
     state = super(Polyhedron_normaliz, self).__getstate__()
     # Remove the unpicklable entries.
     del state[1]['_normaliz_cone']
     return state

This constructs an object just as

sage: P = P.base_extend(P.base_ring(),backend='field')
sage: P.base_extend(P.base_ring(),backend='normaliz')

(but saving computed results)

However, one would need a method to recover _normaliz_cone (this method is needed anyway, to make the second thing work).

@kliem
Copy link
Contributor

kliem commented Oct 19, 2019

comment:4

I think I know what to do about it.

  1. As mentioned one can just remove the cone on pickling. Then the loaded object is just as good as changing backend back and forth (and changing backend to normaliz should also work and still give us a cone or a way to retrive the cone).

  2. Next step would be do allow initialization of a cone from Vrepresentation and Hrepresentation. This works by homogenization of the input and explicitly giving a dehomogenization (this is the only way that Normaliz accepts precomputed data).

    Note: I'm not proposing to allow to give both representations to normaliz by the user, but when changing fields or loading a stored object I think we should trust them to be correct.

  3. Once this is done, one can set up normaliz cone to be a lazy attribute.

@kliem
Copy link
Contributor

kliem commented Oct 19, 2019

Dependencies: #28639

@kliem
Copy link
Contributor

kliem commented Oct 19, 2019

comment:5

In #28639 I will implement a method that generates the cone from both Vrep and Hrep (recomputing the lines, but thats ok I guess). I have tested this with a few polyhedra, but I have no idea, which examples can be tricky.

@kliem

This comment has been minimized.

@kliem kliem modified the milestones: sage-8.4, sage-9.0 Oct 21, 2019
@kliem
Copy link
Contributor

kliem commented Dec 2, 2019

New commits:

c42c907method to obtain cone from Vrep and Hrep
cc17f07added documentation to cone from normaliz data
a54cfd9Merge branch 'public/28639' of git://trac.sagemath.org/sage into public/28639-reb
fc4c596alignment fix in docs
f42a4e2removed pickling restriction on normaliz tests
984cc62fixed pickling/unpickling of normaliz polyhedra

@kliem
Copy link
Contributor

kliem commented Dec 2, 2019

Commit: 984cc62

@kliem
Copy link
Contributor

kliem commented Dec 2, 2019

Author: Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Dec 2, 2019

Branch: public/26363

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2019

Changed commit from 984cc62 to a75d3b2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

a75d3b2take care of special cases

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Dec 2, 2019

New commits:

a75d3b2take care of special cases

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2019

Changed commit from a75d3b2 to 12b9060

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 2, 2019

Branch pushed to git repo; I updated commit sha1. New commits:

12b9060fixing optional flags, added doctest for number field polyhedron pickling

@w-bruns
Copy link
Mannequin

w-bruns mannequin commented Dec 2, 2019

comment:11

I plan to extend Normaliz as suggested. The essential point is that Normaliz can skip a convex hull computation, but nevertheless can produce the facet/ectreme ray incidence vectors and keep them for further operations, like the modification of the original cone.

I am not sure whether one can give up the requirement to input the precomputed data in homogenized form with an added dehomogenization if they come from an inhomogeneous computation.

@embray
Copy link
Contributor

embray commented Jan 6, 2020

comment:12

Ticket retargeted after milestone closed

@embray embray modified the milestones: sage-9.0, sage-9.1 Jan 6, 2020
@kliem
Copy link
Contributor

kliem commented Jan 27, 2020

Changed branch from public/26363 to public/26363-reb

@kliem
Copy link
Contributor

kliem commented Jan 27, 2020

Changed commit from 12b9060 to 57beae3

@kliem
Copy link
Contributor

kliem commented Jan 27, 2020

New commits:

7681b24removed pickling restriction on normaliz tests
5a9decffixed pickling/unpickling of normaliz polyhedra
38dff79take care of special cases
57beae3fixing optional flags, added doctest for number field polyhedron pickling

@tscrim
Copy link
Collaborator

tscrim commented Jan 29, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Jan 29, 2020

comment:14

If there are any extensions to Normaliz that can be utilized in the future (as per comment:11), we can do further changes then. This is a nice improvement. LGTM.

@vbraun
Copy link
Member

vbraun commented Jan 31, 2020

Changed branch from public/26363-reb to 57beae3

@w-bruns
Copy link
Mannequin

w-bruns mannequin commented Feb 2, 2020

Changed commit from 57beae3 to none

@w-bruns
Copy link
Mannequin

w-bruns mannequin commented Feb 2, 2020

comment:16

Meanwhile I have implemented the use of precomputed data. Version 3.8.4 will very soon be published. Normaliz accepts

Type::extreme_rays

and

Type::support_hyperplanes

as precomputed data.

Howevber, these do not always define the cone (or polyhedron) in which we want to compute since nontrivial coordinate transformations may come into play. These are restored via

Type::generated_lattice

(also in the algebraic case) and

Type::maximal_subspace

It is also important that these precomputed data are considered homogeneous input types so that the polyhedron must be defined via Type::dehomogenization or Type::grading.

See Sections 6.21 and D.8.16 in Normaliz.pdf (as of today).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants