sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.36k stars 462 forks source link

Sublattice fan isomorphism bug #16012

Open novoselt opened 10 years ago

novoselt commented 10 years ago

Fans living in sublattices are not correctly checked for being isomorphic as demonstrated in a doctest of #12892.

CC: @sagetrac-jakobkroeker

Component: geometry

Keywords: toric

Branch/Commit: u/jkeitel/sub_fan_isomorphism @ 656fa89

Issue created by migration from https://trac.sagemath.org/ticket/16012

a1031e5c-5e2c-4c90-8ba4-91be67e304df commented 10 years ago
comment:2

Hi Andrey,

there's a simple fix for that. Simply replace

if self.lattice_dim() == 2:

in fan.py by

if self.lattice_dim() == 2 and self.lattice() is self.lattice().ambient_module() and other.lattice() is other.lattice().ambient_module()

because the alternative method of trying to find an isomorphism still works - it's just the echelon forms that 'know' about the sublattice and therefore compare unequal. Alternatively, one could modify the fan_2d_echelon_form() in fan_isomorphism.py such that it removes empty rows.

What do you think?

a1031e5c-5e2c-4c90-8ba4-91be67e304df commented 10 years ago
comment:3

Okay, actually it might be smarter to do something else, because although what I suggested above fixes the is_isomorphic method, the isomorphism method still does not work.

I am attaching a branch that changes the output of two auxiliary functions. When computing echelonized forms of 2d fans and when trying to find isomorphisms between fans, the coordinates now used aren't those of the ambient space anymore, but of a basis of the lattice. I'm not sure whether that's a smart thing to do, since there might be some arbitrariness in the choice of basis, but FanMorphism expects a matrix with nrows / ncols equal to the dimension of the lattice itself, not the dimension of the ambient space.

What do you think? In any case, there are still some doctests missing.


New commits:

656fa89Isomorphisms between fans in sublattices
a1031e5c-5e2c-4c90-8ba4-91be67e304df commented 10 years ago

Commit: 656fa89

a1031e5c-5e2c-4c90-8ba4-91be67e304df commented 10 years ago

Branch: u/jkeitel/sub_fan_isomorphism

ea1d0bf8-c27a-4548-8cb7-de0b1d02441a commented 7 years ago
comment:5

Does this issue still hold? Could someone provide a failing example?

novoselt commented 7 years ago
comment:6

Well, the test from the posted commit fails:

sage: N = ToricLattice(3)
sage: S = N.submodule([(1,-1,0), (1,1,1)])
sage: B = S.basis()
sage: cones = [Cone([B[0], B[1]]), Cone([B[1], -B[0]-B[1]]), Cone([-B[0]-B[1], B[0]])]
sage: f = Fan(cones)
sage: f2 = toric_varieties.P(2).fan()
sage: from sage.geometry.fan_isomorphism import find_isomorphism
sage: find_isomorphism(f, f2)
---------------------------------------------------------------------------
FanNotIsomorphicError                     Traceback (most recent call last)
<ipython-input-9-45f2d7a96da1> in <module>()
----> 1 find_isomorphism(f, f2)

/home/novoselt/sage/local/lib/python2.7/site-packages/sage/geometry/fan_isomorphism.pyc in find_isomorphism(fan1, fan2, check)
    243         m = next(generator)
    244     except StopIteration:
--> 245         raise FanNotIsomorphicError
    246 
    247     from sage.geometry.fan_morphism import FanMorphism

FanNotIsomorphicError: 

while in fact the isomorphism is given by the identity matrix here.