Opened 12 years ago

Closed 12 years ago

#8709 closed defect (fixed)

polyhedra level set doctest failure

Reported by: John Palmieri Owned by: mhampton
Priority: major Milestone: sage-4.4
Component: geometry Keywords:
Cc: mhampton, Andrey Novoseltsev Merged in: sage-4.4.alpha1
Authors: Marshall Hampton Reviewers: Andrey Novoseltsev
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

On one platform (an itanium machine running red hat), I see this:

File "/home/palmieri/cleo/sage-4.4.alpha0/devel/sage/sage/geometry/polyhedra.py", line 3147:
    sage: for lset in polytopes.cross_polytope(2).face_lattice().level_sets(): print lset[0]
Expected:
    (None, (0, 1, 2, 3))
    ((1,), (2, 3))
    ((1, 2), (3,))
    ((0, 1, 2, 3), None)
Got:
    (None, (0, 1, 2, 3))
    ((0,), (1, 2))
    ((1, 2), (3,))
    ((0, 1, 2, 3), None)

This is with Sage 4.4.alpha0, and it comes from the patch in ticket #8650.

Attachments (2)

trac_8709.patch (1.8 KB) - added by mhampton 12 years ago.
apply instead of 8650 patch
trac_8709_fix_sorting_issue_from_8650_on_itanium.patch (1.1 KB) - added by Andrey Novoseltsev 12 years ago.
Apply only this one.

Download all attachments as: .zip

Change History (9)

Changed 12 years ago by mhampton

Attachment: trac_8709.patch added

apply instead of 8650 patch

comment:1 Changed 12 years ago by Andrey Novoseltsev

It looks like some sorting issue. I personally think that it would be nice to have some standard sorting of faces (and it is nice to have this sorting documented).

LatticePolytope's sort faces of dimension 0 according to the generating vertices (so that the i-th 0-dimensional face is generated by the i-th vertex) and codimension 1 faces according to the containing facets (so that the i-th codimension-1 face is contained in the i-th facet). I think that these ways of sorting are very natural and it would be nice to have the same for Polyhedra with its V- and H-representations. For other faces there is probably no "canonical" choice, but it would be nice to have something fixed. (Middle dimensional faces of LatticePolytope's are sorted using one of the above ways, depending on working with an "initial polytope" or its polar - this is done to preserve polar duality of faces for reflexive polytopes.)

comment:2 Changed 12 years ago by mhampton

Status: newneeds_review

The attached patch avoids this sorting issue by only examining the extremes of the face lattice, which is where the problems from 8650 were.

comment:3 Changed 12 years ago by Andrey Novoseltsev

Authors: Marshall Hampton
Reviewers: Andrey Novoseltsev
Status: needs_reviewpositive_review

The code in the patch is the same as in #8650, therefore it passes all other doctests just as well as before. The sorting issue is not an issue anymore with the modified doctest for these changes. So positive review, but it still would be nice to add some sorting in the future, especially for dim=0 and codim=1 faces.

comment:4 Changed 12 years ago by John Palmieri

I'm confused: how is this possibly going to apply over the patch at #8650 (which has been merged into 4.4.alpha0)?

comment:5 Changed 12 years ago by Andrey Novoseltsev

Oops, good point. I am attaching a new patch that just replaces the doctest. Can I still leave it at positive review?..

Changed 12 years ago by Andrey Novoseltsev

Apply only this one.

comment:6 in reply to:  5 Changed 12 years ago by John Palmieri

Replying to novoselt:

Oops, good point. I am attaching a new patch that just replaces the doctest. Can I still leave it at positive review?..

Yes.

comment:7 Changed 12 years ago by John Palmieri

Merged in: sage-4.4.alpha1
Resolution: fixed
Status: positive_reviewclosed

Merged "trac_8709_fix_sorting_issue_from_8650_on_itanium.patch" into 4.4.alpha1.

Note: See TracTickets for help on using tickets.