Oscar.jl icon indicating copy to clipboard operation
Oscar.jl copied to clipboard

ToricVarieties: Rays are being shuffled

Open HereAround opened this issue 2 years ago • 8 comments

Give a list r of rays (and of the maximal cones) to the constructor of a polyhedral fan. This results in a fan F, but rays(F) and r are typically not identical but shuffled. This is very relevant when the input rays have a particular interpretation. E.g., in the standard constructor del_pezzo, it is assumed that the rays of the blow-up coordinates are at the end of the list rays(F) and those for the P2 at its beginning. Hence, the names for the homogeneous coordinates that come with this standard constructor are only meaningful if that is indeed the case. Otherwise, they are (more or less) non-sense.

This shuffling should be taken into account for the constructor del_pezzo but possibly in other places as well.

(I hope to open a PR shortly which attempts to fix this, by making changes to ToricVarieties. A possibly nicer solution would be a fan constructor, which only shuffles the rays for internal speed-ups, but leaves the order unchanged for the "outside" world.)

HereAround avatar Mar 24 '22 18:03 HereAround

What if the input is faulty, i.e. has duplicate rays, rays that are inside cones, and rays that aren't used?

lkastner avatar Mar 24 '22 19:03 lkastner

I assume(d) sane input.

The changes that I propose for del_pezzo are here: https://github.com/oscar-system/Oscar.jl/pull/1209/commits/59409e80ed2dca8c08c131c31b044c5a30f3561e

HereAround avatar Mar 24 '22 19:03 HereAround

I assume(d) sane input.

This is really a tricky one. Because if we allow such constructors then people will abuse them. We have that a lot in polymake, where people specify e.g. VERTICES instead of POINTS and get unexpected results.

This issue also appears in other parts of OSCAR I assume, for example when you give an ideal via generators, probably with different constructors for groups, etc., which is why I ask @fingolfin and @thofma on their take.

lkastner avatar Mar 24 '22 20:03 lkastner

On a side note: I am all for providing convenience functions for facilitating the translation.

lkastner avatar Mar 24 '22 20:03 lkastner

Adding @alexej-jordan for an estimate of how much work this would be for the polyhedral section. This would require work on all polyhedral objects, it does not make sense to only touch fans.

lkastner avatar Mar 24 '22 20:03 lkastner

Good question! I am afraid that we are not quite consistent, but there are many places where we do keek the raw input around:

julia> Qx, (x,) = PolynomialRing(QQ, ["x"])
(Multivariate Polynomial Ring in x over Rational Field, fmpq_mpoly[x])

julia> ideal(Qx, [x, x, x])
ideal(x, x, x)

julia> gens(ans)
3-element Vector{fmpq_mpoly}:
 x
 x
 x

I believe there are functions whose output depends on the generators. So in essence, for us ideals are ideals together with an ordered list of generators (although our notion of equality does not take the generators into account). There are other objects in number theory where this 'problem' occurs. CC: @fieker

If I remember correctly the groups also remember the generators? @fingolfin can probably say more about this.

thofma avatar Mar 25 '22 06:03 thofma

On Thu, Mar 24, 2022 at 12:40:24PM -0700, Lars Kastner wrote:

What if the input is faulty, i.e. has duplicate rays, rays that are inside cones, and rays that aren't used?

There are different notions: stupid and wrong many constructors check, by default, for wrong-ness unless , check=false is specified. Examples are number_field which has to be created with an irreducible this is checked. Similar, orders have to be generated by integral elements, ...

So far we do not check (or correct) for stupid, ie. having redundant generators for ideals (x,x,x) is fine. Depending on the ring they are

  • stored as is
  • processed anyway (ie. number fields: ideals have 1, 2 or n generators and the 2 case is regulated.)

So there is no rule for stupidity, but strong precendences for wrong-ness

-- Reply to this email directly or view it on GitHub: https://github.com/oscar-system/Oscar.jl/issues/1208#issuecomment-1078041296 You are receiving this because you are subscribed to this thread.

Message ID: @.***>

fieker avatar Mar 25 '22 07:03 fieker

@lkastner mentioned that there is a difference between specifying e.g. POINTS and VERTICES as input. We already utilize this for some objects/constructions within Oscar, like the constructor for Cone, by having an optional argument non_redundant::Bool. Is there a reason why we can not use this for fans (or derived objects) to fix the rays and maximal cones together with their order?

alexej-jordan avatar Apr 11 '22 14:04 alexej-jordan