simplicial complex method for polytopes

sagemath / sage

Main repository of SageMath

https://www.sagemath.org

Other

1.32k stars 451 forks source link

simplicial complex method for polytopes #6077

Closed 5d2aaf09-c963-473a-bf79-1f742a72700f closed 15 years ago

5d2aaf09-c963-473a-bf79-1f742a72700f commented 15 years ago

This just adds a simplicial complex method for the polytope class, which requires lrs to compute a triangulation.

CC: @jhpalmieri

Component: geometry

Keywords: polytopes, simplicial

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

5d2aaf09-c963-473a-bf79-1f742a72700f commented 15 years ago

Attachment: trac_6077_polysimpcomplex.patch.gz

adds a simplicial_complex method to polytopes

jhpalmieri commented 15 years ago

comment:2

Mostly good, but needs a few fixes. I'm attaching a referee's patch which does the following:

removes the 'verbose' argument, since it's not used
inserts a warning about this possibly failing if the dimension is larger than 3
inserts '# optional' flags on the doctests so that they don't fail on machines without lrs installed

mhampton's patch gets a positive review from me; only my patch needs to be reviewed. With my patch, all tests pass on sage.math (without lrs installed). With lrs installed, sage -t -optional passes on this file. (But someone else should probably double-check this.)

jhpalmieri commented 15 years ago

apply on top of other patch

5d2aaf09-c963-473a-bf79-1f742a72700f commented 15 years ago

Attachment: 6077-referee.patch.gz

5d2aaf09-c963-473a-bf79-1f742a72700f commented 15 years ago

comment:3

Attachment: 6077_v2.patch.gz

Thanks for looking at this. Let me apologize for being somewhat confused about it - I have code in development for doing triangulations with lrs, but currently Sage doesn't use this. So the warning about possibly not working in high dimensions is good, but I can also delete the check for lrs until I merge that code in.

Anyway, I have added a new patch "6077_v2.patch" which does not depend on the others and should account for the above comments and corrections.

jhpalmieri commented 15 years ago

comment:4

Looks good to me. Passes all tests (without lrs installed, which shouldn't be relevant anymore).

85eec1a4-3d04-4b4d-b711-d4db03337c41 commented 15 years ago

comment:5

Merged 6077_v2.patch only in Sage 4.0.rc0. In case that was not intended please let me know.

Cheers,

Michael