Opened 11 years ago

Closed 11 years ago

#12361 closed defect (fixed)

Fix normalization in cohomology ring of orbifold toric varieties

Reported by: Volker Braun Owned by: Alex Ghitza
Priority: major Milestone: sage-5.1
Component: algebraic geometry Keywords:
Cc: Andrey Novoseltsev, David Eklund Merged in: sage-5.1.beta0
Authors: Volker Braun Reviewers: David Eklund, Andrey Novoseltsev
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

For simplicial toric varieties, the rational cohomology ring and the ratironal Chow group are isomorphic. So if all normalizations are correct, then one should be able to do intersection computations in the cohomology ring. This patch fixes the volume_class() and the constructor of cohomology cycles from cones to make everything work.

It turns out that index_in_saturation does not work for trivial zero-size matrices, this is also fixed.

Apply trac_12361_fix_toric_cohomology_ring.patch, trac_12361_index_in_saturation_fix.patch

Attachments (2)

trac_12361_index_in_saturation_fix.patch (1.5 KB) - added by Volker Braun 11 years ago.
Initial patch
trac_12361_fix_toric_cohomology_ring.patch (21.1 KB) - added by Volker Braun 11 years ago.
Updated patch

Download all attachments as: .zip

Change History (17)

Changed 11 years ago by Volker Braun

Initial patch

comment:1 Changed 11 years ago by Volker Braun

Cc: Andrey Novoseltsev added
Description: modified (diff)
Status: newneeds_review

comment:2 Changed 11 years ago by Andrey Novoseltsev

Reviewers: Andrey Novoseltsev

The patchbot complains about added whitespace! Looking at the actual code...

comment:3 Changed 11 years ago by Andrey Novoseltsev

That's a really great expansion of the documentation!

I think I caught some typos:

  • In toric_variety line 213 should be "many quantities of interest" without "s" on the end.
  • On line 1977 "It gets tricky none of the maximal cones is smooth." does not sound right, some words or punctuation sighs are missing.

comment:4 Changed 11 years ago by Andrey Novoseltsev

Status: needs_reviewneeds_work
Work issues: whitespaces and typos

OK, other than the above looks good!

comment:5 Changed 11 years ago by David Eklund

Hi. I looked at this and I agree that it looks good.

One thing:

On line 215 of toric_variety.py it says "For toric varieties with at most orbifold singularities, the rational cohomology ring H(X,\QQ) and the rational Chow ring H(X,\QQ) are isomorphic."

It's a bit confusing that they are both denoted by H(X,\QQ). Maybe A(X,\QQ) could be used for the Chow ring. Or H^*(X,\QQ) for cohomology and H_*(X,\QQ) for the Chow ring (if intersection homology and the Chow ring were already identified somehow).

comment:6 Changed 11 years ago by David Eklund

Cc: David Eklund added

comment:7 Changed 11 years ago by Volker Braun

Oh yes good catch I meant to write A(X,\QQ) of course.

comment:8 Changed 11 years ago by Volker Braun

Status: needs_workneeds_review

Fixed!

comment:9 Changed 11 years ago by Andrey Novoseltsev

Reviewers: Andrey NovoseltsevDavid Eklund, Andrey Novoseltsev
Status: needs_reviewneeds_work

Comments 2&3 are still applicable and the patchbot complains ;-)

Changed 11 years ago by Volker Braun

Updated patch

comment:10 Changed 11 years ago by Volker Braun

I've improved the docstrings.

As for the whitespace, I think this is a non-issue. There was some discussion on sage-devel and the consensus seems to be that its not worth the effort. Its a button press away (either with emacs or with the mercurial checkfiles plugin) to strip all trailing whitespace, but that would just break every patch we currently have. Mercurial tells me that there are 170 places in toric_varieties.py with superfluous spaces, so a few more or less doesn't matter. I'd rather not spend an hour fixing all patches that I currently have just to make the whitespace plugin happy.

comment:11 Changed 11 years ago by Volker Braun

Status: needs_workneeds_review

comment:12 Changed 11 years ago by Andrey Novoseltsev

Status: needs_reviewpositive_review

comment:13 Changed 11 years ago by Andrey Novoseltsev

Work issues: whitespaces and typos

comment:14 Changed 11 years ago by Jeroen Demeyer

Description: modified (diff)
Milestone: sage-5.0sage-5.1

comment:15 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-5.1.beta0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.