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: |
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)
Change History (9)
Changed 12 years ago by
Attachment: | trac_8709.patch added |
---|
comment:1 Changed 12 years ago by
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
Status: | new → needs_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
Authors: | → Marshall Hampton |
---|---|
Reviewers: | → Andrey Novoseltsev |
Status: | needs_review → positive_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
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 follow-up: 6 Changed 12 years ago by
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
Attachment: | trac_8709_fix_sorting_issue_from_8650_on_itanium.patch added |
---|
Apply only this one.
comment:6 Changed 12 years ago by
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
Merged in: | → sage-4.4.alpha1 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
Merged "trac_8709_fix_sorting_issue_from_8650_on_itanium.patch" into 4.4.alpha1.
apply instead of 8650 patch