Opened 10 years ago

Closed 10 years ago

#9926 closed defect (fixed)

Doctest error in sage/schemes/generic/toric_divisor.py on OS X

Reported by: mpatel Owned by: mvngu
Priority: blocker Milestone: sage-4.6
Component: doctest coverage Keywords:
Cc: novoselt, vbraun, mhampton Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

I get this reproducible doctest error with a trial 4.6.alpha1 (32-bit build) on bsd.math (OS X 10.6):

sage -t -long "devel/sage/sage/schemes/generic/toric_divisor.py"
**********************************************************************
File "/Users/mpatel/tmp/bb/slave/bsd_scratch/build/sage-4.6.alpha1/devel/sage/sage/schemes/generic/toric_divisor.py", line 1522:
    sage: supp.Vrepresentation()
Expected:
    [A vertex at (-1, 1), A vertex at (0, 2), A vertex at (0, -1), A vertex at (3, -1)]
Got:
    [A vertex at (-1, 1), A vertex at (0, 2), A vertex at (3, -1), A vertex at (0, -1)]

Change History (21)

comment:1 Changed 10 years ago by mpatel

If it's feasible to create a patch now, I can merge it into 4.6.alpha1, while I wait for a response to a build error at #4000.

comment:2 Changed 10 years ago by mpatel

By the way, the unofficial, trial 4.6.alpha1 is in /home/release/sage-4.6.alpha1 on the Sage cluster.

comment:3 Changed 10 years ago by novoselt

Volker, what's the correct way to sort vertices? And what does their order in Vrepresentation depend on?

comment:4 Changed 10 years ago by vbraun

The output order should be deterministic: _sheaf_cohomology_support() does nothing that could randomize the order of chamber_vertices and Polyhedron takes the ordering of the vertices to be whatever cddlib returns. I don't have access to an OSX machine nor an account on bsd.math, so someone else will have to debug this.

comment:5 Changed 10 years ago by novoselt

  • Cc mhampton added

Marshall, is there any chance you can settle this, since I don't understand the guts of Polyhedron? Does cddlib have any specific ordering of vertices? (IMHO, taking convex hull should delete extra points and otherwise leave the order the same.) Even if cddlib does not care about any particular order I find it strange that it can depend on architecture...

Of course, an easy fix to this is to put "#random output" comment in the doctest, but it does not feel satisfactory in this case.

comment:6 Changed 10 years ago by mhampton

I'll try to check this out but I don't have much time today, it might take me a few.

comment:7 Changed 10 years ago by jhpalmieri

This is not as pleasant, but what about something like:

sage: all([v in supp.vertices() for v in [[-1, 1], [0, 2], [3, -1], [0, -1]]])
True

comment:8 Changed 10 years ago by mhampton

That would work, although as an example it seems harder to understand. I think I would prefer:

sage: vertices = supp.vertices()
sage: vertices.sort()
sage: vertices
[[-1, 1], [0, -1], [0, 2], [3, -1]]

at least as a temporary workaround. It would be nice if the Polyhedron class produced a specific output, but that will require some significant additions that I won't have time for before this release.

comment:9 Changed 10 years ago by vbraun

I think the problem is that cddlib uses rand() to sometimes make random initial choices while searching for generating sets. Although cddlib fixes the seed, there is no guarantee that different platforms implement rand() in the same way.

To make sure this is indeed the problem, can you run

sage: Polyhedron(vertices=[(0, 1), (1, 1), (0, 1), (-1, 1), (0, 2), (0, -1), (0, 0), (0, 0), (3, -1), (0, 2), (0, -1), (1, -1), (0, 0)], verbose=True)
V-representation
begin
 13 3 rational
 1 0 1
 1 1 1
 1 0 1
 1 -1 1
 1 0 2
 1 0 -1
 1 0 0
 1 0 0
 1 3 -1
 1 0 2
 1 0 -1
 1 1 -1
 1 0 0
end

V-representation
begin
 4 3 rational
 1 -1 1
 1 0 2
 1 0 -1
 1 3 -1
end

H-representation
begin
 4 3 rational
 2 1 -1
 1 2 1
 1 0 1
 2 -1 -1
end
A 2-dimensional polyhedron in QQ^2 defined as the convex hull of 4 vertices.

on the OSX machine and paste the output? I can then rip out the rand() from the cddlib.spkg...

comment:10 Changed 10 years ago by mhampton

  • Owner changed from mvngu to mhampton

I'm confused by "rip out the rand() from the cddlib.spkg" - are you going to patch the source code?

Anyway on OS X 10.6 I get:

V-representation
begin
 13 3 rational
 1 0 1
 1 1 1
 1 0 1
 1 -1 1
 1 0 2
 1 0 -1
 1 0 0
 1 0 0
 1 3 -1
 1 0 2
 1 0 -1
 1 1 -1
 1 0 0
end


V-representation
begin
 4 3 rational
 1 -1 1
 1 0 2
 1 3 -1
 1 0 -1
end

H-representation
begin
 4 3 rational
 2 1 -1
 1 2 1
 1 0 1
 2 -1 -1
end

Vertex graph
begin
  4    4
 1 2 : 2 4 
 2 2 : 1 3 
 3 2 : 2 4 
 4 2 : 1 3 
end

Facet graph
begin
  4    4
 1 2 : 2 4 
 2 2 : 1 3 
 3 2 : 2 4 
 4 

comment:11 Changed 10 years ago by mhampton

  • Owner changed from mhampton to mvngu

comment:12 follow-up: Changed 10 years ago by vbraun

Yes, thats what I'm planning to do: patch the cddlib source code to use a portable random number generator. I already have an updated spkg for #9798, so I'll patch this issue at the same time if you don't object.

comment:13 Changed 10 years ago by dimpase

same failure on PPC MacOSX 10.5, just in case... Dima

comment:14 follow-up: Changed 10 years ago by vbraun

  • Status changed from new to needs_info

Updated spkg is at #9798, please test on your OSX machines. Note: also needs patch to the sage library which you can find on the same ticket.

comment:15 in reply to: ↑ 14 Changed 10 years ago by dimpase

  • Status changed from needs_info to needs_work

Replying to vbraun:

Updated spkg is at #9798, please test on your OSX machines. Note: also needs patch to the sage library which you can find on the same ticket.

I just did, please see the other ticket... Dima

comment:16 in reply to: ↑ 12 Changed 10 years ago by jason

Replying to vbraun:

Yes, thats what I'm planning to do: patch the cddlib source code to use a portable random number generator. I already have an updated spkg for #9798, so I'll patch this issue at the same time if you don't object.

I'm curious; are you planning on using Sage's framework for interfacing to random number generators? See sage/devel/sage/sage/misc/randstate.pyx, and examples for setting the random seed in gap, pari, the libc generator, etc., or see http://sagemath.org/doc/reference/sage/misc/randstate.html#generating-random-numbers-in-library-code

comment:17 Changed 10 years ago by vbraun

My plan is described in #10039. So I'm not interested in adding more to cddlib than what we absolutely have to.

comment:18 Changed 10 years ago by mpatel

Update: I also get the problem in the description with a 64-bit build of 4.6.alpha3 on bsd.math (OS X 10.6).

comment:19 follow-up: Changed 10 years ago by vbraun

  • Status changed from needs_work to needs_review

I've fixed the bug in #9798. That ticket will fix the issue in this ticket, too.

comment:20 in reply to: ↑ 19 Changed 10 years ago by dimpase

  • Status changed from needs_review to positive_review

Replying to vbraun:

I've fixed the bug in #9798. That ticket will fix the issue in this ticket, too.

tested on Debian x64 and on MacOSX 10.5 PPC. Positive!

comment:21 Changed 10 years ago by mpatel

  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.