Skip to content

Commit 9e60de4

Browse files
author
Release Manager
committed
gh-40387: Refactor Hilbert_basis() and replace a slow test Initially my goal was to replace one slow `Hilbert_basis()` test, but I did some refactoring along the way -- none of which really affects the performance of the Hilbert basis calculation. The commit message lists these changes. To replace the test, I made up some examples and fed them to normaliz. If the sage answer agrees with the normaliz answer, they must both be right, right? URL: #40387 Reported by: Michael Orlitzky Reviewer(s): Martin Rubey, Michael Orlitzky
2 parents 0eb0fa7 + 58ce4c4 commit 9e60de4

File tree

1 file changed

+79
-59
lines changed

1 file changed

+79
-59
lines changed

src/sage/geometry/cone.py

Lines changed: 79 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,6 +1682,13 @@ def _contains(self, point, region='whole cone'):
16821682
raise ValueError("%s is an unknown region of the cone!" % region)
16831683
if region == "interior" and self.dim() < self.lattice_dim():
16841684
return False
1685+
if self.is_trivial():
1686+
# We always have whole cone = relative interior = {0} for
1687+
# the trivial cone, and in addition we have interior = {0}
1688+
# when the ambient space is trivial (the preceding "if"
1689+
# ensures this).
1690+
return point.is_zero()
1691+
16851692
need_strict = region.endswith("interior")
16861693
M = self.dual_lattice()
16871694
for c in self._PPL_cone().minimized_constraints():
@@ -4369,7 +4376,7 @@ def Hilbert_basis(self):
43694376
N(1, 1)
43704377
in 2-d lattice N
43714378
4372-
Two more complicated example from GAP/toric::
4379+
A more complicated example from GAP/toric::
43734380
43744381
sage: Cone([[1,0], [3,4]]).dual().Hilbert_basis()
43754382
M(0, 1),
@@ -4378,38 +4385,51 @@ def Hilbert_basis(self):
43784385
M(2, -1),
43794386
M(3, -2)
43804387
in 2-d lattice M
4381-
sage: cone = Cone([[1,2,3,4], [0,1,0,7], [3,1,0,2], [0,0,1,0]]).dual()
4382-
sage: cone.Hilbert_basis() # long time
4383-
M(10, -7, 0, 1),
4384-
M(-5, 21, 0, -3),
4385-
M( 0, -2, 0, 1),
4386-
M(15, -63, 25, 9),
4387-
M( 2, -3, 0, 1),
4388-
M( 1, -4, 1, 1),
4389-
M( 4, -4, 0, 1),
4390-
M(-1, 3, 0, 0),
4391-
M( 1, -5, 2, 1),
4392-
M( 3, -5, 1, 1),
4393-
M( 6, -5, 0, 1),
4394-
M( 3, -13, 5, 2),
4395-
M( 2, -6, 2, 1),
4396-
M( 5, -6, 1, 1),
4397-
M( 8, -6, 0, 1),
4398-
M( 0, 1, 0, 0),
4399-
M(-2, 8, 0, -1),
4400-
M(10, -42, 17, 6),
4401-
M( 7, -28, 11, 4),
4402-
M( 5, -21, 9, 3),
4403-
M( 6, -21, 8, 3),
4404-
M( 5, -14, 5, 2),
4405-
M( 2, -7, 3, 1),
4406-
M( 4, -7, 2, 1),
4407-
M( 7, -7, 1, 1),
4408-
M( 0, 0, 1, 0),
4409-
M( 1, 0, 0, 0),
4410-
M(-1, 7, 0, -1),
4411-
M(-3, 14, 0, -2)
4412-
in 4-d lattice M
4388+
4389+
Examples verified independently using [Normaliz]_::
4390+
4391+
sage: cones.rearrangement(3,4).Hilbert_basis()
4392+
N(-2, 1, 1, 1),
4393+
N( 1, 1, 1, -2),
4394+
N( 1, 1, -2, 1),
4395+
N( 1, -2, 1, 1),
4396+
N( 1, 0, 0, 0),
4397+
N(-1, 1, 1, 0),
4398+
N(-1, 1, 0, 1),
4399+
N( 0, -1, 1, 1),
4400+
N( 1, 0, -1, 1),
4401+
N( 1, 0, 1, -1),
4402+
N(-1, 0, 1, 1),
4403+
N( 0, 1, -1, 1),
4404+
N( 1, -1, 0, 1),
4405+
N( 0, 1, 1, -1),
4406+
N( 1, -1, 1, 0),
4407+
N( 0, 1, 0, 0),
4408+
N( 1, 1, 0, -1),
4409+
N( 1, 1, -1, 0),
4410+
N( 0, 0, 1, 0),
4411+
N( 0, 0, 0, 1)
4412+
in 4-d lattice N
4413+
4414+
sage: # long time
4415+
sage: K = Cone([(1,0,1), (-1,0,1), (0,1,1), (0,-1,1)])
4416+
sage: K.Hilbert_basis()
4417+
N( 1, 0, 1),
4418+
N(-1, 0, 1),
4419+
N( 0, 1, 1),
4420+
N( 0, -1, 1),
4421+
N( 0, 0, 1)
4422+
in 3-d lattice N
4423+
4424+
sage: # long time
4425+
sage: K = Cone([(1,0,1,0), (-1,0,1,0), (0,1,1,0), (0,-1,1,0)])
4426+
sage: K.Hilbert_basis()
4427+
N( 1, 0, 1, 0),
4428+
N(-1, 0, 1, 0),
4429+
N( 0, 1, 1, 0),
4430+
N( 0, -1, 1, 0),
4431+
N( 0, 0, 1, 0)
4432+
in 4-d lattice N
44134433
44144434
Not a strictly convex cone::
44154435
@@ -4436,41 +4456,41 @@ def Hilbert_basis(self):
44364456
44374457
The primal Normaliz algorithm, see [Normaliz]_.
44384458
"""
4439-
if self.is_strictly_convex():
44404459

4441-
def not_in_linear_subspace(x):
4442-
return True
4443-
else:
4444-
linear_subspace = self.linear_subspace()
4460+
# Used in lieu of our linear_subspace() for containment
4461+
# testing when this cone is strictly convex. None of the
4462+
# points we test can be zero, so checking the empty tuple is
4463+
# equivalent to checking a trivial subspace, and the empty
4464+
# tuple is faster.
4465+
L = ()
44454466

4446-
def not_in_linear_subspace(x):
4447-
# "x in linear_subspace" does not work, due to absence
4448-
# of coercion maps as of Github issue #10513.
4449-
try:
4450-
linear_subspace(x)
4451-
return False
4452-
except (TypeError, ValueError):
4453-
return True
4467+
if not self.is_strictly_convex():
4468+
# Our linear_subspace(), but as a cone, so that
4469+
# containment testing using "in" works properly.
4470+
L = Cone((c*r for c in (1, -1) for r in self.lines()),
4471+
self.lattice(),
4472+
check=False)
44544473

44554474
irreducible = list(self.rays()) # these are irreducible for sure
4456-
gens = list(self.semigroup_generators())
4457-
for x in irreducible:
4458-
try:
4459-
gens.remove(x)
4460-
except ValueError:
4461-
pass
4475+
irr_modified = False # have we appended to "irreducible"?
4476+
gens = [x for x in self.semigroup_generators()
4477+
if x not in irreducible]
44624478

4479+
from itertools import chain
44634480
while gens:
44644481
x = gens.pop()
4465-
if any(not_in_linear_subspace(y) and x-y in self
4466-
for y in irreducible+gens):
4467-
continue
4468-
irreducible.append(x)
4469-
if len(irreducible) == self.nrays():
4470-
return self.rays()
4471-
else:
4482+
if all((y in L) or (x-y not in self)
4483+
for y in chain(irreducible, gens)):
4484+
irreducible.append(x)
4485+
irr_modified = True
4486+
4487+
if irr_modified:
44724488
return PointCollection(irreducible, self.lattice())
44734489

4490+
# Avoid the PointCollection overhead if nothing was
4491+
# added to the irreducible list beyond self.rays().
4492+
return self.rays()
4493+
44744494
def Hilbert_coefficients(self, point, solver=None, verbose=0,
44754495
*, integrality_tolerance=1e-3):
44764496
r"""

0 commit comments

Comments
 (0)