Change gitfan from polymake to Oscar types

[jankoboehm]

This is the first round of changes to adapt from polymake types to Oscar types. Everything works except the last step to integrate fans with groups actions. Some int types should be updated still.

[fingolfin]

The error I see in the logs is this:

Examples.GITFans, the example: Error During Test at /home/runner/work/Oscar.jl/Oscar.jl/test/Experimental/GITFans-test.jl:1
  Got exception outside of a @test
  ArgumentError: Unrecognized argument type: Cone{QQFieldElem}.
  You need to convert to polymake compatible type first.
  Stacktrace:
    [1] convert_to_pm_type(T::Type)
      @ Polymake ~/.julia/packages/Polymake/A0Z5B/src/convert.jl:64
    [2] convert(#unused#::Type{Polymake.PolymakeType}, x::Cone{QQFieldElem})
      @ Polymake ~/.julia/packages/Polymake/A0Z5B/src/convert.jl:29
    [3] _broadcast_getindex_evalf
      @ ./broadcast.jl:670 [inlined]

[jankoboehm]

The last step with the new group action on fans has to be still put together properly.

[fingolfin]

Some remarks. Not sure if some of this is covered by "The last step with the new group action on fans has to be still put together properly", but I guess it doesn't hurt to mention these?

[fingolfin]

Maybe this would also work now in the much nicer new setup? It would arguably easier to understand, I think

Suggested change

	if ! any(j -> j == cone,
	collector_cones)
	if !(cone in collector_cones)

[fingolfin]

Dunno if this is supported, but if it is, IMHO it would be easier to read

Suggested change

	if contains(current_cone, point)
	if point in current_cone

[lkastner]

I added that to our todolist, but I think it makes sense to do this in a different PR.

[fingolfin]

Could also write it like this:

Suggested change

	!any(f->contains(f, facet_points[i]), faces(q_cone, dim(q_cone)-1)) || continue
	!any(contains(facet_points[i]), faces(q_cone, dim(q_cone)-1)) || continue

and if #2142 is resolved then could even write

Suggested change

	!any(f->contains(f, facet_points[i]), faces(q_cone, dim(q_cone)-1)) || continue
	!any(contains(facet_points[i]), facets(Cone, q_cone)) || continue

[lkastner]

This requires more work than suggested in #2142:

julia> c = positive_hull([1 0; 0 1])
Polyhedral cone in ambient dimension 2

julia> fc = facets(Cone{QQFieldElem}, c)
2-element SubObjectIterator{Cone{QQFieldElem}}:
 Polyhedral cone in ambient dimension 2
 Polyhedral cone in ambient dimension 2

julia> [rays_modulo_lineality(f) for f in fc]
2-element Vector{NamedTuple{(:rays_modulo_lineality, :lineality_basis), Tuple{SubObjectIterator{RayVector{QQFieldElem}}, SubObjectIterator{RayVector{QQFieldElem}}}}}:
 (rays_modulo_lineality = [[1, 0]], lineality_basis = [[0, 1]])
 (rays_modulo_lineality = [[0, 1]], lineality_basis = [[1, 0]])

As you can see, these "facets" are just the linear halfspaces as a different type. I was just as confused as you and happy that I found a workaround for now. I am currently discussing with @benlorenz what to do.

[fingolfin]

Thank you for looking into this :-)

[fingolfin]

OK, I see:

julia> q_cone = cone_from_inequalities([-1 0 0; 0 -1 0; 0 0 -1])
Polyhedral cone in ambient dimension 3

julia> f1 = faces(q_cone, dim(q_cone)-1);

julia> f2 = facets(Cone{QQFieldElem}, q_cone);

julia> [rays_modulo_lineality(f) for f in f1]
3-element Vector{NamedTuple{(:rays_modulo_lineality, :lineality_basis), Tuple{SubObjectIterator{RayVector{QQFieldElem}}, SubObjectIterator{RayVector{QQFieldElem}}}}}:
 (rays_modulo_lineality = [[0, 1, 0], [0, 0, 1]], lineality_basis = [])
 (rays_modulo_lineality = [[1, 0, 0], [0, 0, 1]], lineality_basis = [])
 (rays_modulo_lineality = [[1, 0, 0], [0, 1, 0]], lineality_basis = [])

julia> [rays_modulo_lineality(f) for f in f2]
3-element Vector{NamedTuple{(:rays_modulo_lineality, :lineality_basis), Tuple{SubObjectIterator{RayVector{QQFieldElem}}, SubObjectIterator{RayVector{QQFieldElem}}}}}:
 (rays_modulo_lineality = [[1, 0, 0]], lineality_basis = [[0, 1, 0], [0, 0, 1]])
 (rays_modulo_lineality = [[0, 1, 0]], lineality_basis = [[1, 0, 0], [0, 0, 1]])
 (rays_modulo_lineality = [[0, 0, 1]], lineality_basis = [[1, 0, 0], [0, 1, 0]])

So yeah, these are different. Thanks also for the explanation on gather.

[fingolfin]

I think this is not used anymore (good!)

Suggested change

q_cone_facets_converted = matrix(QQ, q_cone.pm_cone.FACETS)

[fingolfin]

There's a lot of improvements here. Any reason not to merge this right now? We can still make further improvements in a new PR afterwards, right?

@jankoboehm if you agree, please remove the draft status, please until tomorrow.