Opened 9 years ago

Closed 9 years ago

#12965 closed defect (fixed)

X.Kaehler_cone().lattice() is not a lattice

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

Status badges


In Sage-5.0 we have

sage: toric_varieties.P2().Kaehler_cone().lattice()
The toric rational divisor class group of a 2-d CPR-Fano toric variety covered by 3 affine patches
sage: _.base_ring()
Rational Field

which is wrong since this lattice is not a lattice.

Attachments (1)

trac_12965_rational_class_group_lattice.patch (6.9 KB) - added by novoselt 9 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 9 years ago by novoselt

Hi Volker,

Any thoughts on what would be the proper way to fix it? Modify Cone constructor to take as the lattice of a cone in a vector space the integral span of the basis and normalize/convert rays to live in this span?

It seems to be correct, but it screws up printing where these rays live in - this integral span shows as row span of the identity matrix without mention of any class groups. It also means that rays cannot be lifted to divisors, without first converting them to the rational class group explicitly.

A half-way solution is to use span of the basis for normalization of rays, but let rays be still elements of the rational divisor class group. However, in this case, it seems that rays of a cone in the N-lattice must have N_R as their parent and if we let rays live in the extension all the customization work of toric lattices becomes irrelevant.

So I am inclining to the first solution - if Cone gets a rational vector space V for the lattice, it actually works with span(ZZ, V.gens()).

Let me know what you think!

comment:2 Changed 9 years ago by novoselt

Another thought: instead of rational divisor class group have free divisor class group (does it have an actual name?) which is just the class group without torsion. As I understand, this is exactly the Z-span of the basis of the current one.

Changed 9 years ago by novoselt

comment:3 Changed 9 years ago by novoselt

  • Authors set to Andrey Novoseltsev
  • Dependencies set to #13023
  • Status changed from new to needs_review

Here is a more or less minimal solution, just a ZZ-module with the same elements.

comment:4 Changed 9 years ago by novoselt

  • Keywords sd40.5 added

comment:5 Changed 9 years ago by vbraun

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

Looks good to me!

comment:6 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.1 to sage-5.2

comment:7 Changed 9 years ago by jdemeyer

  • Dependencies #13023 deleted

comment:8 Changed 9 years ago by jdemeyer

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