oscar-system/jtoric.jl

jtoric.jl's Issues

Cones in constructor of NormalToricVariety

Should one declare the cones as matrix? I.e. cones::Matrix{Int}.

(Cf. #34)

-> No.

Constructors for toric varieties

• toric_variety( fan )
• toric_varieties( rays, max_cones )
• toric_varieties( polytope ) -> always projective
• toric_varieties( vertices, edges ) -> always projective
• toric_varieties( Coxring ) (-> Spec( R ) )
• toric_varieties from triangulations

Replace structs by oscar data types

• E.g. use polynomial rings, so that aboslute primary decomposition can be used (cf. https://oscar-system.github.io/Oscar.jl/dev/CommutativeAlgebra/ideals/#Decomposition-of-Ideals)
• Use Z^n grading and report back to oscar.

All structs must use CamelCase

(not camelCase - cf. #38)

A bunch of remarks and suggestions

Thank you for working on this! I have some minor feedback, after just browsing your source code. Some of them may be nonsense or won't work, as I only read the code, did not try any o this.

1. why export CapAndHomalg and export GAP?
2. why do you sometimes write CapAndHomalg.GAP.Globals instead of GAP.Globals?
3. in particular CapAndHomalg.GAP.Globals.ConvertJuliaToGAP(foobar) seems odd -- I suggest to instead call julia_to_gap(foobar) or even better, GapObj(foobar)
4. similarly, don't call GAP.Globals.GAPToJulia, use gap_to_julia instead -- or even better, if you know the result type, then use that. I.e. if x is a GAP string, then it's better to write String(x) than gap_to_julia(x), as that allows for reliable type inference, makes the code easier to understand, and adds a layer of protection against mistakes
5. most (all?) of your structs lack type annotations, so everything is stored as Any. That prevents optimizations and may also waste storage. Type annotations also help people reading your code to understand it better, and of course they help catch some errors
6. your code formatting is a bit weird. Why does each function start and end with an empty line?
7. the names you export do not conform to Julia / Oscar naming guidelines: IsCartier should be iscartier, etc.
8. why do many (all?) of your structs have an unused member bar?
9. I recommend replacing Array{T,1} with the equivalent Vector{T}.
10. we just released Oscar.jl 0.6.0, so you may want to adjust your Project.toml accordingly

Regarding the conversion functions, it may also help to use that GAP.jl can convert things recursively. For example, let's look at this function:

function ListOfVariablesOfCoxRing( v::JToricVariety,  )
    
    gap_variables = CapAndHomalg.GAP.Globals.ListOfVariablesOfCoxRing( v.GapToricVariety )
    len = GAP.Globals.GAPToJulia( GAP.Globals.Length( gap_variables ) )
    julia_variables = [  GAP.Globals.GAPToJulia( gap_variables[ i ] ) for i in 1:len ]
    return julia_variables
    
end

I think it could be rewritten to something like this (but I did not verify this; and I only guessed a what type those gap_variable[i] are supposed to have):

function ListOfVariablesOfCoxRing(v::JToricVariety)
    vars = CapAndHomalg.GAP.Globals.ListOfVariablesOfCoxRing(v.GapToricVariety)
    return Vector{String}(vars)
end

Or for another example, consider this:

function IsProductOf( v::JToricVariety )
    
    factors = CapAndHomalg.GAP.Globals.IsProductOf( v.GapToricVariety )
    len = GAP.Globals.GAPToJulia( GAP.Globals.Length( factors ) )
    j_factors = [ JToricVariety( 1, factors[ i ] ) for i in 1:len ]
    return j_factors
    
end

It should be possible to rewrite this as follows:

function IsProductOf(v::JToricVariety)
    factors = CapAndHomalg.GAP.Globals.IsProductOf(v.GapToricVariety)
    return [ JToricVariety( 1, f ) for f in factors ]
end

Conventions

• Remove all "J"s
• Check redundant method IsNormalVariety
• Function names: all small (no capitals) and underscore as separator

Remove extra white spaces from examples/tests.

As pointed out by @fingolfin (cf. #38).

JConvex: get rid of `Polymake_V_Rep` and `Polymake_H_Rep`?

For various computations, a new GAP object is created via Polymake_V_Rep and Polymake_H_Rep. But that really shouldn't be necessary, as Polymake is happy to start with a h-rep or v-rep object which then can be queried for the other rep.

Add support for more Polymake functionality

• Gorenstein
• Q-Gorenstein
• Fano
• Nef cone
• Mori cone

Use jldoctest to test examples

As pointed out by @fingolfin (cf. #38):

You could also use jldoctest, then these examples would also be run as part of the doctest step of the CI (provided that is set up), which has the advantage that the examples are tested, so one knows they actually are working and correct. But of course sometimes this is not desirable, e.g. if outputs are random or differ between Julia versions. Seems to be safe here, though?

Regarding "randomness": The only one that comes to my mind is the one discussed here: #27 I did not see this happen recently, so maybe this is already resolved?

merge JToric.jl into Oscar

JConvex: remove remaining `JuliaEvalString` and `EvalString` uses

see homalg-project/ToricVarieties_project#140

Resolve random (?) failures in CI tests

CI tests in various Julia versions (I've seen 1.4, 1.5, 1.6) have been failing. This usually goes away after restarting. Maybe a race condition?

Here's a backtrace of one such failure; unfortunately it is not very useful:

ERROR: Package JToric errored during testing
 [1] pkgerror(msg::String)
   @ Pkg.Types /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Pkg/src/Types.jl:55
 [2] test(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; coverage::Bool, julia_args::Cmd, test_args::Cmd, test_fn::Nothing)
   @ Pkg.Operations /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Pkg/src/Operations.jl:1693
 [3] test(ctx::Pkg.Types.Context, pkgs::Vector{Pkg.Types.PackageSpec}; coverage::Bool, test_fn::Nothing, julia_args::Cmd, test_args::Cmd, kwargs::Base.Iterators.Pairs{Symbol, IOContext{Base.PipeEndpoint}, Tuple{Symbol}, NamedTuple{(:io,), Tuple{IOContext{Base.PipeEndpoint}}}})
   @ Pkg.API /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Pkg/src/API.jl:343
 [4] test(pkgs::Vector{Pkg.Types.PackageSpec}; io::IOContext{Base.PipeEndpoint}, kwargs::Base.Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:coverage,), Tuple{Bool}}})
   @ Pkg.API /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Pkg/src/API.jl:80
 [5] test(; name::Nothing, uuid::Nothing, version::Nothing, url::Nothing, rev::Nothing, path::Nothing, mode::Pkg.Types.PackageMode, subdir::Nothing, kwargs::Base.Iterators.Pairs{Symbol, Bool, Tuple{Symbol}, NamedTuple{(:coverage,), Tuple{Bool}}})
   @ Pkg.API /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/Pkg/src/API.jl:96
 [6] top-level scope
   @ none:1

Move to organization

@fingolfin @lkastner: For efficient collaboration, I think you should have sufficient permissions (write, merge etc.). Does it make sense to migrate this repository to Oscar?

I have done similar transitions in the past. It should be possible to hand over this private repository to an organization (e.g. Oscar). In particular, this transition should (?) also preserve issues, PRs etc.

Intersection theory

We should have methods to compute topological intersection numbers. Specifically, I am thinking of smooth, complete, normal toric varieties of dimension N. For toric divisors D1, D2, ..., DN, the topological intersection number D1 * D2 * ... * DN is then a (finite) integer, which we should be able to compute.

Extensions would be towards orbifolds, with typically rational intersection numbers (if well-defined). Another extension is to support operations with the full intersection rings. This would start by having support for the Cox ring and its quotient by the sum of the Stanley-Reisner ideal and the ideal of linear relations.

Upstream NConvex patches so that we can use an official version

Right now, JToric uses a patched version of NConvex in order to allow the JConvex GAP package to override the NConvex code calling Normaliz/Cddlib, by code which calls Polymake (through Julia / Polymake.jl) instead.

We don't want this on the long run. Rather, we want to work with @kamalsaleh and @mohamed-barakat to get changes into NConvex that allow us to use that again. There are several ways to do this:

1. we could merge JConvex into NConvex and then NConvex could be somehow configured to use either Normaliz/Cddlib, or to use JConvex
2. we could change NConvex to always load (even without NormalizInterface/CddInterface present); it'd always load its general infrastructure, but would load the code specific to normaliz/cdd if those are present. (That's effectively what @HereAround's patched version currently does)
3. one could move the common code to a separate package (perhaps reactivate the Convex package for that?) and then one can have multiple "providers" of actual implementations.

Option 3 sounds the cleanest on paper, but it's also most work, I think, and it's not clear who would make the decision which "provider" of functionality to actually load, and when. This is solved better with 1 and 2. The drawback of 1 is that we are currently still changing JConvex a lot; and ideally I think we should have it inside the JToric.jl repository, so that we can change it in lock step with the rest of JToric. So from that point of view, option 2 would be best, except if not done carefully, it'd degrade the user experience for people using "regular" GAP (for those ideally nothing would change).

So here's a slightly crazy idea: as a slight variant of approach (2), the PackageInfo.g of NConvex could be changed to look something like this:

# if running GAP under Julia and JConvex is around, then use that
if IsPackageMarkedForLoading("JuliaInterface", "") and TestPackageAvailability("JConvex") then
   nconvex_needed := [ [ "JConvex"  ] ];
else
   nconvex_needed := [ [ "CddInterface", ">= 2020.06.24" ],
                           [ "NormalizInterface", ">= 1.1.0"  ] ];
fi;

SetPackageInfo( rec(
PackageName := "NConvex",
...
Dependencies := rec(
  GAP := ">= 4.9",
  NeededOtherPackages := Concatenation([ [ "AutoDoc", ">= 2018.02.14" ],
                           [ "Modules", ">= 0.5" ], 
                           [ "CddInterface", ">= 2020.06.24" ],
                           [ "NormalizInterface", ">= 1.1.0"  ]
                         ],
...
));
Unbind(nconvex_needed);
```

JConvex: systematically deal with rational and big coefficients

When I refactored JConvex, I replaced code like this:

        s := JuliaToGAP( IsString, Julia.string( cone!.pmobj.RAYS ) );
        res_string := SplitString( s, '\n' );
        res_string := List( [ 2 .. Length( res_string ) ], i -> Concatenation( "[", ReplacedString( res_string[ i ], " ", "," ), "]" ) );
        rays := EvalString( Concatenation( "[", JoinStringsWithSeparator( res_string, "," ), "]" ) );

by stuff like this:

        rays := JuliaToGAP( IsList, JuliaMatrixInt( cone!.pmobj.RAYS ) );

where JuliaMatrixInt really is the Julia type Matrix{Int}.

I think that was a mistake. We probably should be using Matrix{Rational{BigInt}}. Because JConvex then often proceeds with code like this:

        # sometimes, Polymake returns rational rays - we turn them into integral vectors
        scaled_rays := [];
        for i in [ 1 .. Length( rays ) ] do
            scale := Lcm( List( rays[ i ], r -> DenominatorRat( r ) ) );
            Append( scaled_rays, [ scale * rays[ i ] ] );
        od;

From @lkastner I learned that one can let Polymake do this scaling, by calling Polymake.common.primitive. So perhaps even better would be to replace all that scaling code by something like that (note the use of a to-be-defined JuliaMatrixBigInt):

        rays := _Polymake_jl.common.primitive( cone!.pmobj.RAYS ); # ensure integer coeffs
        rays := JuliaToGAP( IsList, JuliaMatrixBigInt( rays ) );

and then `scaled_rays is superfluous.

Rename this repository to JToric.jl?

Julia convention suggests this, and the Julia package manager requires this, I think (if this package ever is to be registered).

Deploy documentation

https://docs.julialang.org/en/v1/manual/documentation/

Support data types for different kinds of toric varieties

Say: AffineToricVariety, ProjectiveToricVariety, NormalToricVariety, ...

Add constructors for Hirzebruch and del Pezzo surfaces

Points to be addressed

1. Are all instances of using GAP and using CapAndHomalg strictly necessary? In particular, they currently appear (at least) once in every file in the src-folder.
2. Similarly, a few instances of CapAndHomalg.LoadPackage( "ToricVarieties" ) (and the same for JConvex and JuliaInterface) might be redundant (replace by GAP.LoadPackage() or alike?)
3. Sometimes (I cannot reliably reproduce this) the version number is correctly detected and sometimes not. Identify the cause.
4. Regroup the content of the src-folder to resemble the content of https://github.com/homalg-project/ToricVarieties_project.
5. Extend tests to Julia 1.3 and latest.

Toric Varieties from triangulations

If we want the physics community to be interested, then we must allow to construct toric varieties from triangulations.

Typical example: Given a polytope, work out all of its FRSTs (fine, regular, star triangulations) and return a list/array of the toric varieties that are build from the different triangulations.

Test suite fails for me while trying to `EvalString` some GAP code

So clearly this is a local problem as the CI tests pass here. But I am wondering what might be wrong here!

I already tried running Pkg.build( "JToric" ) again. No such luck. Perhaps @HereAround has a hint

Precompiling project...
  1 dependency successfully precompiled in 28 seconds (86 already precompiled)
     Testing Running tests...
Welcome to JToric 0.1.0
Enjoy this software to perform computations on toric geometry!
JToric.jl: Error During Test at /Users/mhorn/Projekte/OSCAR/JToric/test/runtests.jl:16
  Test threw exception
  Expression: JToric.is_isomorphic_to_projective_space(H5) == false
  Error thrown by GAP: Syntax error: identifier expected in stream:1
  _EVALSTRINGTMP:={0};
                   ^
  Syntax error: -> expected in stream:1
  �
  Error, Could not evaluate string.
   at /Users/mhorn/.julia/artifacts/8f22bf2645df36936a8a74cea9de55d6070017ed/share/gap/lib/string.gi:658 called from
  EvalString( res_string[i] ) at /Users/mhorn/.julia/packages/CapAndHomalg/PE2rB/pkg/ToricVarieties_project/JConvex/gap/ExternalPolymakeCone.gi:442 called from
  func( C[i] ) at /Users/mhorn/.julia/artifacts/8f22bf2645df36936a8a74cea9de55d6070017ed/share/gap/lib/coll.gi:665 called from
  List( [ 2 .. Length( res_string ) ], function ( i )
  return EvalString( res_string[i] );
  end ) at /Users/mhorn/.julia/packages/CapAndHomalg/PE2rB/pkg/ToricVarieties_project/JConvex/gap/ExternalPolymakeCone.gi:442 called from
  Polymake_RaysInFacets( ExternalPolymakeCone( cone ) ) at /Users/mhorn/.julia/packages/CapAndHomalg/PE2rB/pkg/ToricVarieties_project/JConvex/gap/Cone.gi:112 called from
  RaysInFacets( cone ) at /Users/mhorn/.julia/gaproot/v4.12/pkg/NConvex/gap/Cone.gi:613 called from
  ...  at *defin*:0

  Stacktrace:
   [1] error(s::String)
     @ Base ./error.jl:33
   [2] ThrowObserver(depth::Int32)
     @ GAP ~/.julia/packages/GAP/9XZl9/src/GAP.jl:63
   [3] _call_gap_func(func::GAP.GapObj, a1::GAP.GapObj)
     @ GAP ~/.julia/packages/GAP/9XZl9/src/ccalls.jl:255
   [4] call_gap_func_nokw
     @ ~/.julia/packages/GAP/9XZl9/src/ccalls.jl:222 [inlined]
   [5] (::GAP.GapObj)(a1::GAP.GapObj)
     @ GAP ~/.julia/packages/GAP/9XZl9/src/ccalls.jl:232
   [6] is_isomorphic_to_projective_space(v::NormalToricVariety)
     @ JToric ~/Projekte/OSCAR/JToric/src/ToricVarieties.jl:337
   [7] macro expansion
     @ ~/Projekte/OSCAR/JToric/test/runtests.jl:16 [inlined]
   [8] macro expansion
     @ /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/Test/src/Test.jl:1151 [inlined]
   [9] top-level scope

Possible Road map for toric varieties

The following is a list of tasks that I believe would be interesting to look into for toric varieties.

Of course, I do not claim that this list is complete, neither that the points need to be addressed in this order. Some of the points might also be debatable/need to be sharpened. Still, I hope that this gives a first impression of a possible roadmap.

I would be happy to hear what people think about this.

• Coordinate ring of torus
• List of variables of coordinate ring of torus
• The rational function of a character
• Cartier divisors, in particular collectively the group of Cartier torus invariant divisors
• Support polymake's CARTIER_DATA
• Cache computed properties:
• More sophisticated show methods for toric varieties and toric divisors,
• Allow the user to choose the variable names of the Cox ring and the coordinate ring of the torus to their liking. (For example, I would like to label homogeneous coordinates introduced by a blowup by e.)
• Remember genesis of a toric variety, e.g. if it was obtained as direct product and if so, apply refined algorithms.
• Remember all toric divisors created for a toric variety and cache their properties as well.
• Comparison among toric varieties (e.g. isprojective_space, isdirect_product_of_projective_space, isdelPezzo etc.).
• Construct toric varieties from triangulations (likely via topcom).
• Construct toric varieties from GLSM charges.
• Support Cox variety.
• Toric morphisms.
• Line bundles:
• Picard group,
• cohomologies via cohomCalg or chamber counting,
• use cohomCalg to identify vanishing sets (key for many algorithms in the ToricVarieties_project).
• Support famous vector bundles:
• Tangent and cotangent bundle of toric varieties.
• Normal and tangent bundle of complete intersections.
• Compute resolutions and use cohomCalg to try to compute their cohomologies. (Will of course not always work, but is a quick first check.)
• Extend to coherent sheaves and their cohomologies (many algorithms in the ToricVarieties_project).
• Intersection theory in the Chow ring.

Integrate package JConvex

https://github.com/homalg-project/ToricVarieties_project/tree/master/JConvex

toric_ideal_binomial_generators for normal toric varieties

Methods for AffineNormalToricVariety shoud also be callable by a NormalToricVariety v in case isaffine( v ) == true. However, the following leads to an error:

C = Oscar.positive_hull([1 1; -1 1])
ntv = NormalToricVariety(C)
isaffine( ntv )
toric_ideal_binomial_generators( ntv )

@lkastner Suggestions?

Avoid usage of GAP/Polymake wrappers

Eventually, we should directly ask polymake about fans/cones, rather than using its wrapper for ToricVarieties (avoiding code duplication/simplification). This can probably be considered as an improvement of the code style.

[For now postponed in favour of adding functionality.]

(Cf. #34)

Improve calling of Polymake

As suggested by @fingolfin in #38, properly call/import Polymake in JToric:

• Extend the __init__ of JToric by JToric.GAP.Globals.pm = Polymake (pm to be replaced by __Polymake or __PolymakeFromJulia or so or POLYMAKE_JULIA_MODULE or whatever, just to avoid clashes with user objects.
• Then make calls to Polymake via pm.prefered_function.