ToricVarieties: Faster Stanley-Reisner ideal, (default) coefficient ring, names for homogeneous variables and fmpz-valued rays from polymake

[HereAround]

This PR aims towards resolving the discussion in #932. In the current form, I find this draft NOT satisfying. I am certain that other people will find more issues. Here is what I see at this point:

As @fieker reported, there seem to be an issue with the following function whenever we have large/huge rays:

function map_from_character_to_principal_divisors(v::AbstractNormalToricVariety)
mat = transpose(Matrix{Int}(Polymake.common.primitive(pm_object(v).RAYS)))
matrix = AbstractAlgebra.matrix(ZZ, mat)
return hom(character_lattice(v), torusinvariant_divisor_group(v), matrix)
end

Specifically, the conversion into Matrix{Int} can then fail. Generally speaking, it should be possible to replace this line via rays(fan(v)). This is currently not possible, because the return value is then Polymake.Rationals, while one should expect to have fmpz. As a temporary fix, I added a corresponding function in 01d8f48. @lkastner and @micjoswig: What are your thoughts on this?

To compute the Stanley-Reisner ideal via a simplicial complex, we can first focus on projective varieties, so that the fan is the normal fan of a polyhedron P. We do not have a function which computes this polyhedron yet. Is there functionality for this somewhere? If yes, I could not (yet) find it. A temporary fix is to cache the polyhedron, if the variety is created from one: 4abd464

Next, one could hope to call stanley_reisner_ideal(S,K) , with S the graded (!) Cox ring and K the simplicial complex in question. This is not possible because currently only non-graded rings over Z are accepted as input:

function stanley_reisner_ideal(R::FmpzMPolyRing, K::SimplicialComplex)
n = nvertices(K)
return ideal([ R([1], [_characteristic_vector(f,n)]) for f in minimal_nonfaces(K) ])
end

In particular, the expression ideal([ R([1], [_characteristic_vector(f,n)]) for f in minimal_nonfaces(K) ]) cannot seem to be evaluated for a graded ring R. Ring experts, please comment.

The following change would work, but is probably not robust enough (see 10cd389):

function stanley_reisner_ideal(R::MPolyRing, K::SimplicialComplex)
n = nvertices(K)
collections = [findall(x->x==1, _characteristic_vector(f,n)) for f in minimal_nonfaces(K)]
vars = Hecke.gens(R)
generators = [prod(vars[I]) for I in collections]
return ideal(generators)
end

Finally, I change the homogeneous coordinate from x[i] to xi. This looks nicer, but has other drawbacks (e.g. as they cannot be generated programmatically as @fieker pointed out). The relevant commits are:

• f5d0c4e: Change x[i] to xi.
• 328646a: An attempt to label the blowup coordinate as ei. (Not sufficiently robust at this point, in my opinion.)
• 17cf7a1: A method to allow the user to choose variable names for the Cox ring as he/she pleases. @fieker: I believe you had something more global in mind.

Last but not least, it would appear that Z, Q as base rings have each advantages/disadvantages. No resolution for this yet.

Finally, to rebase more easily, this PR contains the changes of #929 and #940, which I hope are both a lot less critical and can be merged very soon.

[HereAround]

@thofma I just force pushed and thereby accidentally removed the commit with your suggested change. I amended it to the latest commit, but the "co-authored by" text is gone. I could try to add this by hand if you care. Sorry about that.

[thofma]

No worries :)

[fieker]

You could replace FmpzMPolyRing by 'MPolyRing{fmpz}', try supertype(FmpzMPolyRing) followed by subtypes(ans). This will cover both graded and non-graded rings.
The QQ vs ZZ: it really depends what you want to do with it. Any classical commutative algebra (Groebner and friends) and you're sunk when using ZZ. Combinatorcs and easyly moving to FF_p, then ZZ is better.
The fact that the generators of the ideal are in ZZ[x] is irrelevant as this is true for all ideals...

[fieker]

On Mon, Jan 03, 2022 at 08:44:13AM -0800, Martin Bies wrote: @HereAround commented on this pull request. > + +# Examples +```jdoctest +julia> p2 = NormalToricVariety(normal_fan(Oscar.simplex(2))) +A normal, non-affine, smooth, projective, gorenstein, q-gorenstein, fano, 2-dimensional toric variety without torusfactor + +julia> set_cox_ring(p2, "u1, u2, u3") + +julia> cox_ring(p2, "u1, u2, u3") +(Multivariate Polynomial Ring in u1, u2, u3 over Rational Field graded by + u1 -> [0 0 1] + u2 -> [0 0 1] + u3 -> [0 0 1], MPolyElem_dec{fmpq, fmpq_mpoly}[u1, u2, u3]) +``` +""" +function set_cox_ring(v::AbstractNormalToricVariety, var_list::Vector{String}) Thank you!

Cannot work, "u1, u2, u3" is of wrong type

…

-- Reply to this email directly or view it on GitHub: #939 (comment) You are receiving this because you are subscribed to this thread. Message ID: ***@***.***>

[HereAround]

On Mon, Jan 03, 2022 at 08:44:13AM -0800, Martin Bies wrote: @HereAround commented on this pull request. > + +# Examples +jdoctest +julia> p2 = NormalToricVariety(normal_fan(Oscar.simplex(2))) +A normal, non-affine, smooth, projective, gorenstein, q-gorenstein, fano, 2-dimensional toric variety without torusfactor + +julia> set_cox_ring(p2, "u1, u2, u3") + +julia> cox_ring(p2, "u1, u2, u3") +(Multivariate Polynomial Ring in u1, u2, u3 over Rational Field graded by + u1 -> [0 0 1] + u2 -> [0 0 1] + u3 -> [0 0 1], MPolyElem_dec{fmpq, fmpq_mpoly}[u1, u2, u3]) + +""" +function set_cox_ring(v::AbstractNormalToricVariety, var_list::Vector{String}) Thank you!
Cannot work, "u1, u2, u3" is of wrong type
…
-- Reply to this email directly or view it on GitHub: #939 (comment) You are receiving this because you are subscribed to this thread. Message ID: @.***>

Thank you!

[HereAround]

You could replace FmpzMPolyRing by 'MPolyRing{fmpz}', try supertype(FmpzMPolyRing) followed by subtypes(ans). This will cover both graded and non-graded rings. The QQ vs ZZ: it really depends what you want to do with it. Any classical commutative algebra (Groebner and friends) and you're sunk when using ZZ. Combinatorcs and easyly moving to FF_p, then ZZ is better. The fact that the generators of the ideal are in ZZ[x] is irrelevant as this is true for all ideals...

As already proposed by @micjoswig , it would seem that we should simply allow an additional argument, with which one can specify if the variety is to be constructed over ZZ, QQ, F_p... I propose QQ as default.

[fieker]

Your missing the point, my suggestion allows gradings...

…

On Tue, 4 Jan 2022, 18:05 Martin Bies, ***@***.***> wrote: You could replace FmpzMPolyRing by 'MPolyRing{fmpz}', try supertype(FmpzMPolyRing) followed by subtypes(ans). This will cover both graded and non-graded rings. The QQ vs ZZ: it really depends what you want to do with it. Any classical commutative algebra (Groebner and friends) and you're sunk when using ZZ. Combinatorcs and easyly moving to FF_p, then ZZ is better. The fact that the generators of the ideal are in ZZ[x] is irrelevant as this is true for all ideals... As already proposed by @micjoswig <https://github.com/micjoswig> , it would seem that we should simply allow an additional argument, with which one can specify if the variety is to be constructed over ZZ, QQ, F_p... I propose QQ as default. — Reply to this email directly, view it on GitHub <#939 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA36CV4MBLTBLLEA3RF3EALUUMSD5ANCNFSM5LFRGOQQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you commented.Message ID: ***@***.***>

[micjoswig]

The fact that the generators of the ideal are in ZZ[x] is irrelevant as this is true for all ideals...

Well, truth is, the quotient is usually more important than the ideal; so there is a difference. I am absolutely in favor of picking QQ coeffs for (the cohomology rings of) the toric varieties. As far as the Cox ring is concerned, I'd suggest QQ[x1,x2,...] as the default. Of course, we need options for gradings and possibly other rings.

The interface for the simplicial complexes should be similar. While I have a slight preference to make ZZ[x1,x2,...] the default ring, I do not insist. An argument in favor of QQ[x1,x2,...] here, too, would be that this would make it less likely for a user to do something stupid (with respect to computational resources). At any rate, here I do need (at least) the option to specify, e.g., ZZ[x1,x2,...] when I mean it.

[benlorenz]

As @fieker reported, there seem to be an issue with the following function whenever we have large/huge rays:

function map_from_character_to_principal_divisors(v::AbstractNormalToricVariety)
mat = transpose(Matrix{Int}(Polymake.common.primitive(pm_object(v).RAYS)))
matrix = AbstractAlgebra.matrix(ZZ, mat)
return hom(character_lattice(v), torusinvariant_divisor_group(v), matrix)
end

Specifically, the conversion into Matrix{Int} can then fail. Generally speaking, it should be possible to replace this line via rays(fan(v)). This is currently not possible, because the return value is then Polymake.Rationals, while one should expect to have fmpz. As a temporary fix, I added a corresponding function in 01d8f48. @lkastner and @micjoswig: What are your thoughts on this?

The rays of a polyhedral fan or cone are usually considered in QQ (and not in ZZ) as this is required for any convex hull computations, and to retrieve them as a fmpq matrix you can use:

m = vector_matrix(rays(fan(v)))

There is probably some way to eliminate the denominators in a fmpq matrix to get a fmpz matrix (instead of using Polymake.common.primitive) ?

Edit: We could probably also add something like vector_matrix(fmpz, rays(fan(v))) to prescribe the return type.

[HereAround]

I have just updated this PR and hope that it addresses all points and issues made in #932. Hence, I marked it ready for review. Let me summarize the changes:

• Changes to the homogeneous coordinates of the Cox ring: c2bbf87, e3c6670, c2bbf87, cb1c612. In particular, I change them from x[k] to xk and ask for user input to name the exceptional coordinate of a blowup, as suggested by @fieker.
• Support for different coefficient rings as requested by @micjoswig : 05adcef. (I was originally tempted to name this the base_ring, but have decided for coefficient_ring instead once I observed The use of base_ring #938.
• Use fmpz-valued rays of fans: 1926825. This uses the methods recently added by @benlorenz . In particular, this fixes the issue mentioned by @fieker in questions concerning default rings #932. Even more, I added this example as test in c989258).
• Use simplicial complexes to compute the Stanley-Reisner ideal (as pointed out by @micjoswig): 245fa43, 245fa43. Note that there is currently no method to compute the polyhedron of a toric variety (if this exists). Rather, the only way to currently take advantage of this is to construct a variety from a polyhedron. The corresponding constructor then caches the polyhedron. I envision that this can be improved in a future PR, if people are ok with the current state.
• Following the recent improvements of Base.show by @lkastner, I extend the display method of toric varieties further to also display the coefficient ring: 38105b9

cc: @fingolfin and @thofma

[thofma]

No new line or an example :)

[HereAround]

Ok. I will adjust this upon resolving the points mentioned below (regarding two methods, integers as coefficients...). More below.

[thofma]

There are now two methods

    stanley_reisner_ideal(R::MPolyRing, K::SimplicialComplex)

Return the Stanley-Reisner ideal of the abstract simplicial complex `K`, in the given ring `R`.

and

    stanley_reisner_ideal(R::MPolyRing{fmpq}, K::SimplicialComplex)

Return the Stanley-Reisner ideal of the abstract simplicial complex `K` in the given ring `R`.
This method is tailor-made to accept the graded Cox ring of a toric variety.

right? Does the docstring of the first method needs to be updated or do these two methods really have different assumptions on the input?

[micjoswig]

function stanley_reisner_ideal(R::MPolyRing{fmpq}, K::SimplicialComplex)

in

https://github.com/HereAround/Oscar.jl/blob/38105b9075af435679eeb88e20eba51dd52728b1/src/Combinatorics/SimplicialComplexes.jl#L321

Forcing rational coefficients here is unwanted! QQ is the default, and that is fine. But, for my purposes, I need ZZ sometimes.

[HereAround]

@lkastner: I have updated this PR as we discussed earlier this week.

• There exists now an attribute coefficient_ring which can be set via set_coefficient_ring and can be accessed via coefficient_ring(v).
• Likewise, one can set the homogeneous coordinates via set_coordinate_names and get their current value via coordinate_names.
• Both are NOT set upon construction.
• coordinate_names can be changed until cox_ring is computed for the first time. If no choice was made, I invoke the default choice x1, x2, ... and prevent changes from this point on.
• Likewise, coefficient_ring can be changed until either cox_ring or toric_ideal is computed for the first time. If no choice was made until then, the default QQ is invoked. After this point, a change of coefficient_ring is prevented.
• I think that freezing the value for these two attributes is natural at this point and that it prevents possible later inconsistencies. As always, I am happy to be proven wrong/corrected and to update with a better solution.
• If desired, I could add additional convenience methods to compute the ideals/ring with several other coefficient rings. Namely, I imagine a method which would take a ring as additional argument and then return the corresponding ring/ideal. These methods could then be used even after a choice of coefficient_ring/coordinate_names has been fixed. Let me know if this is desired.
• Your point regarding the caching is noted. I believe this PR is complicated already. I suggest to discuss this separately, e.g. in Which properties should be cached where? #997.
• The base.show method was also adjusted. Unless a value for coefficient_ring is known, none is (of course) printed. Once a value is known, it checks for QQ and ZZ (most prominent probably) and then prints "... toric variety over QQ" respectively "... toric variety over ZZ". For other rings, the print-out reads "...over coefficient_ring(variety)". Unfortunately, I could not obtain the value/string of the variable which represent the toric_variety in question in the active julia session via string(v). The latter seems to return the same string as base.show. Hence, I wrote "variety" instead. I hope this is also acceptable.
• The simple command for the simplicial complex that you mentioned above seems to work. This was added in the last commit.

[HereAround]

This needs to be rebased

Done.

[lkastner]

Great work, thank you. Let's discuss everything else after this is merged, since it is very large already as you said.

[lkastner]

@fingolfin Should this then only be merged or squash+merge?

[thofma]

Do the individual commits yield running versions of Oscar and do you want to preserve the individual commits? If yes, merge, otherwise, squash+merge.

[HereAround]

My goal is to make sure that all commits yield a running version. However, due to the long runtime of the tests (in particular the documentation), I have not explicitly checked all commits, but only some during development. Feel free to squash if that seems more reasonable.