Codecov Report

Merging #2228 (9a04931) into master (af045d7) will increase coverage by 0.39%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##           master    #2228      +/-   ##
==========================================
+ Coverage   73.06%   73.46%   +0.39%     
==========================================
  Files         367      367              
  Lines       50072    50091      +19     
==========================================
+ Hits        36587    36798     +211     
+ Misses      13485    13293     -192     

I added a commit 4a033d6 refactoring starsubdivision to use set operations rather than manually converting everything to basically Vector{Bool} (but masqueraded as Vector{Int}) and then setting entries to 0 and 1 manually. Let me know whether this looks correct to you, it already passed the toric_blowup.jl tests on my machine.

I added a commit 4a033d6 refactoring starsubdivision to use set operations rather than manually converting everything to basically Vector{Bool} (but masqueraded as Vector{Int}) and then setting entries to 0 and 1 manually. Let me know whether this looks correct to you, it already passed the toric_blowup.jl tests on my machine.

Thank you @lkastner. I believe this works and looks a lot better than my original version.

I have just rebased this to the new version of #2244 (a couple of doctest failures remained). Hopefully, now most of the tests succeed.

@lkastner I am uncertain how to fix the failure in https://github.com/oscar-system/Oscar.jl/actions/runs/4698570939/jobs/8331022103?pr=2228. Help appreciated.

The new blow_up routine calls _cone_is_smooth(::PolyhedralFan{QQFieldElem}, ::Polymake.SetAllocated{CxxWrap.CxxWrapCore.CxxLong}). On ubuntu-latest, this calls the existing method _cone_is_smooth(::Oscar._FanLikeType, ::AbstractSet{Int64}) (cf. /src/PolyhedralGeometry/PolyhedralFan/properties.jl:593) and succeeds. However, on macOS-latest, this leads to the above failure.

Probably this is a triviality, but I cannot see how to fix this. @fingolfin, @fieker , @thofma - any ideas/suggestions?

It is also failing on macOS with julia 1.6. @benlorenz any idea why this Polymake type pops up?

The reason for this failure is due to the following:

• a polymake Set<Int> is used as an argument and this is derived from AbstractSet{CxxLong} (polymake Int is the same as C++ long)
• on linux CxxLong is an alias for Int64, on macos it is not (this is due to CxxWrap`)

Since the input is converted to a vector anyway loosening the type restriction on the input would probably work, maybe something like AbstractSet{<:Integer} ?

Somewhat unrelated question: Why does the star subdivision require that the cone is smooth?

The reason for this failure is due to the following:

* a polymake `Set<Int>` is used as an argument and this is derived from `AbstractSet{CxxLong}` (polymake `Int` is the same as C++ `long`)

* on linux `CxxLong` is an alias for `Int64`, on macos it is not (this is due to CxxWrap`)

Since the input is converted to a vector anyway loosening the type restriction on the input would probably work, maybe something like AbstractSet{<:Integer} ?

Thank you. I have just changed the input to AbstractSet{<:Integer} . Let's see if the tests pass now.

Somewhat unrelated question: Why does the star subdivision require that the cone is smooth?

This is an assumption in definition 3.3.17 (and 3.3.13) of the toric book by Cox, Little and Schenk. But I suppose your question would then be: "Why is it necessary to make that assumption?". Truth told - I do not (yet) know where this assumption is being used and for what purpose.

I am aware that later in the book, CLS discuss a generalization of this star subdivision which, if my understanding is correct, can also be carried out if the smoothness assumption is violated. Once I get to this (and have a good answer to your question), the smoothness condition could/might be removed/replaced.

In fact, I understand that @apturner wants this eventually for the further development of FTheoryTools. So I hope to get to this generalization sooner than later. But still, one step at a time.

Somewhat unrelated question: Why does the star subdivision require that the cone is smooth?

This is an assumption in definition 3.3.17 (and 3.3.13) of the toric book by Cox, Little and Schenk. But I suppose your question would then be: "Why is it necessary to make that assumption?". Truth told - I do not (yet) know where this assumption is being used and for what purpose.

The definition only works if all the cones are simplicial. Try with the cone over a square and choose an edge to subdivide at, then this does not result in a subdivision of the original fan (the supports disagree). I also think that smoothness is preserved under these operations, but maybe I overlooked something there.