Opened 4 years ago

Closed 4 years ago

#9782 closed defect (fixed)

Enhanced fans fail for complicated cases

Reported by: vbraun Owned by: novoselt
Priority: major Milestone: sage-4.6
Component: algebraic geometry Keywords:
Cc: novoselt Merged in: sage-4.6.alpha1
Authors: Andrey Novoseltsev Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by novoselt)

Fans of toric varieties do not work correctly, for example see the following:

sage: f = Fan([(0, 2, 4), (0, 4, 5), (0, 3, 5), (0, 1, 3), (0, 1, 2), (2, 4, 6), (4, 5, 6), (3, 5, 6), (1, 3, 6), (1, 2, 6)], [(-1, 0, 0), (0, -1, 0), (0, 0, -1), (0, 0, 1), (0, 1, 2), (0, 1, 3), (1, 0, 4)])
sage: f.is_complete()
True
sage: X = ToricVariety(f)
sage: X.fan().is_complete()
---------------------------------------------------------------------------
IndexError                                Traceback (most recent call last)

/home/vbraun/opt/sage-4.5.2/devel/sage-main/<ipython console> in <module>()

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/geometry/fan.pyc in is_complete(self)
   1742         # Then boundary cones are (d-1)-dimensional.
   1743         for cone in self(codim=1):
-> 1744             if len(cone.star_generator_indices()) != 2:
   1745                 self._is_complete = False
   1746                 return False

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/geometry/fan.pyc in __call__(self, dim, codim)
    816         else:
    817             return self.cones(dim, codim)
--> 818 
    819     def __cmp__(self, right):
    820         r"""

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/geometry/fan.pyc in cones(self, dim, codim)
   1547                 if len(top_cones) == self.ngenerating_cones():
   1548                     top_cones.sort(key=lambda cone:
-> 1549                                             cone.star_generator_indices()[0])
   1550                 levels[-1] = top_cones
   1551             if len(levels) >= 2: # We have rays

/home/vbraun/Sage/sage/local/lib/python2.6/site-packages/sage/geometry/fan.pyc in <lambda>(cone)
   1548                     top_cones.sort(key=lambda cone:
   1549                                             cone.star_generator_indices()[0])
-> 1550                 levels[-1] = top_cones
   1551             if len(levels) >= 2: # We have rays
   1552                 rays = list(levels[1])

IndexError: tuple index out of range

I'm pretty sure it is a bug in Andrey's switch to enhanced cones for toric varieties (trac_9470_toric_variety_fans.patch). If I back out that patch it works fine. Unfortunately we didn't catch that in our doctests, we should add tests for complicated toric varieties and mark them as # long time

See #9604 for dependences.

Attachments (1)

trac_9782_bug_in_computing_star_generators.patch (1.9 KB) - added by novoselt 4 years ago.

Download all attachments as: .zip

Change History (7)

comment:1 Changed 4 years ago by novoselt

Working on it!

comment:2 Changed 4 years ago by novoselt

  • Description modified (diff)

comment:3 Changed 4 years ago by novoselt

  • Description modified (diff)

comment:4 Changed 4 years ago by novoselt

  • Authors set to Andrey Novoseltsev
  • Status changed from new to needs_review

It was a silly typo in generic fans (I confused the number of rays with the number of cones). We definitely need more doctests, but so far adding new functionality serves as one of the sources ;-) I have added your example from this ticket to documentation of the function which had the error. It adds about 2 seconds to the test time so I put it in without #long so far - checking completeness uses quite a few functions, so it is a nice one to have.

Anyway - the patch is "one-worder" plus doctest, ready for review!

comment:5 Changed 4 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Thanks, this fixes it and explains why it hasn't surfaced earlier: Tests usually use toric surfaces where the number of rays equals the number of number of generators. But that only holds for 2D complete fans.

comment:6 Changed 4 years ago by mpatel

  • Merged in set to sage-4.6.alpha1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.