Fix blowups along rays and singular cones #4454

paemurru · 2025-01-11T16:31:30Z

Fix blowups along singular cones when the cones is given by an index. Also, fix blowup and star subdivision along a single existing ray in the case where the ray is given by an index among the list of all cones.

This is in the case where the ray is given by the index among the list of all cones. Also, fix star subdivision along an existing ray.

codecov · 2025-01-11T17:39:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.37%. Comparing base (a60b0d8) to head (489204d).
Report is 25 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4454      +/-   ##
==========================================
- Coverage   84.42%   84.37%   -0.05%     
==========================================
  Files         668      672       +4     
  Lines       88363    88717     +354     
==========================================
+ Hits        74600    74855     +255     
- Misses      13763    13862      +99

Files with missing lines	Coverage Δ
...erimental/Schemes/src/ToricBlowups/constructors.jl	`91.89% <100.00%> (+0.84%)`	⬆️
src/PolyhedralGeometry/PolyhedralFan/properties.jl	`98.48% <100.00%> (+0.05%)`	⬆️
...alGeometry/PolyhedralFan/standard_constructions.jl	`100.00% <ø> (ø)`

... and 52 files with indirect coverage changes

experimental/Schemes/src/ToricBlowups/constructors.jl

HereAround · 2025-01-15T13:46:04Z

@paemurru Thank you for working on this. Overall, this looks very good. A few comments:

As stated above, could you point me to the line(s) that fixes the original issue that you face? I am just curious and want to appreciate your work.
My vague recollection is that @lkastner , @HechtiDerLachs and I did not touch on blowups of singular cones. Since you seem to know this scenario well (at least the title of your PR indicates so), could you add a literature reference to the docs, that justifies the computational approach? (Sorry if it is already there and I overlooked it.) Again, this is to fully appreciate your work and verify that the code computes the correct mathematical object.
Modulo "does the code compute the correct mathematical object" (i.e. the previous point), the code changes on the toric varieties side look fine to me.
Regarding the code changes on the polyhedral geometry side, maybe @benlorenz could take a look?

paemurru · 2025-01-15T14:52:40Z

Bugs

There were two bugs before:

(1) Blowing up along a singular cone gave an error. If the intention was to allow only smooth cones, then this should have been checked using an @req and should have been documented.

One fix would have been to add the @req and document it. I took a different approach and implemented blowups along arbitrary nonzero cones.

The code that gave an error before:

julia> ray_generators = [[2, -1], [0, 1]]
2-element Vector{Vector{Int64}}:
 [2, -1]
 [0, 1]
julia> max_cones = IncidenceMatrix([[1, 2]])
1×2 IncidenceMatrix
[1, 2]
julia> X = normal_toric_variety(max_cones, ray_generators)
Normal toric variety
julia> g = blow_up(X, 2)
ERROR: ArgumentError: Cannot subdivide cone 2 as it is generated by a single ray
Stacktrace:
 [1] macro expansion
   @ ~/.julia/packages/AbstractAlgebra/xy6Re/src/Assertions.jl:599 [inlined]
 [2] star_subdivision(Sigma::NormalToricVariety, n::Int64)
   @ Oscar ~/.julia/dev/Oscar/src/PolyhedralGeometry/PolyhedralFan/standard_constructions.jl:255
 [3] blow_up(v::NormalToricVariety, n::Int64; coordinate_name::Nothing)
   @ Oscar ~/.julia/dev/Oscar/experimental/Schemes/src/ToricBlowups/constructors.jl:240
 [4] blow_up(v::NormalToricVariety, n::Int64)
   @ Oscar ~/.julia/dev/Oscar/experimental/Schemes/src/ToricBlowups/constructors.jl:236
 [5] top-level scope
   @ REPL[84]:1

(2) Blowing up along a (single) ray gave an error. The fix was just removing an @req in PolyhedralGeometry, namely

@req length(tau) > 1 "Cannot subdivide cone $n as it is generated by a single ray"

Below is the error message that appeared before.

julia> ray_generators = [[2, -1], [0, 1]]
2-element Vector{Vector{Int64}}:
 [2, -1]
 [0, 1]
julia> max_cones = IncidenceMatrix([[1, 2]])
1×2 IncidenceMatrix
[1, 2]
julia> X = normal_toric_variety(max_cones, ray_generators)
Normal toric variety
julia> f = blow_up(X, 1)
ERROR: ArgumentError: Could not identify position of exceptional ray
Stacktrace:
 [1] macro expansion
   @ ~/.julia/packages/AbstractAlgebra/xy6Re/src/Assertions.jl:599 [inlined]
 [2] (::Oscar.var"#_toric_blowup_morphism#9400")(v::NormalToricVariety, new_variety::NormalToricVariety, coordinate_name::String, exceptional_ray::Vector{…}, center_data::Vector{…})
   @ Oscar ~/.julia/dev/Oscar/experimental/Schemes/src/ToricBlowups/types.jl:38
 [3] Oscar.ToricBlowupMorphism(v::NormalToricVariety, new_variety::NormalToricVariety, coordinate_name::String, exceptional_ray::Vector{…}, center_data::Vector{…}, center_unnormalized::Oscar.ToricIdealSheafFromCoxRingIdeal{…})
   @ Oscar ~/.julia/dev/Oscar/experimental/Schemes/src/ToricBlowups/types.jl:69
 [4] blow_up(v::NormalToricVariety, n::Int64; coordinate_name::Nothing)
   @ Oscar ~/.julia/dev/Oscar/experimental/Schemes/src/ToricBlowups/constructors.jl:243
 [5] blow_up(v::NormalToricVariety, n::Int64)
   @ Oscar ~/.julia/dev/Oscar/experimental/Schemes/src/ToricBlowups/constructors.jl:236
 [6] top-level scope
   @ REPL[83]:1
Some type information was truncated. Use `show(err)` to see complete types.

Documentation

My last commit added the relevant citations to the docstring of blow_up.

Correctness

By Example 11.1.4 in CLS11, the star subdivision along a smooth cone and along the barycenter of the smooth cone coincide.

HereAround

Thank you for elaborating @paemurru . Just looked at your PR again, understanding it much better now. Looks good to me.

I will wait until tomorrow for feedback regarding your changes in the polyhedral geometry (e.g. by @benlorenz). If there are no complaints by the Friday meeting tomorrow, I will merge this PR.

benlorenz

A few small improvements and one correction for the check.

src/PolyhedralGeometry/PolyhedralFan/properties.jl

test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl

src/PolyhedralGeometry/PolyhedralFan/properties.jl

Co-authored-by: Benjamin Lorenz <benlorenz@users.noreply.github.com>

paemurru added 6 commits January 11, 2025 14:29

Fix toric blowups along singular cones

db1b421

JuliaFormatter

443d042

Fix blowups along existing rays

cc6522f

This is in the case where the ray is given by the index among the list of all cones. Also, fix star subdivision along an existing ray.

JuliaFormatter

464c9cf

Improve docstring

4cf4df4

Fix doctest

8288e9a

lgoettgens added topic: polyhedral geometry Issue concerns polyhedral geometry code topic: toric schemes labels Jan 11, 2025

Update docs

a18a5d1

HereAround reviewed Jan 15, 2025

View reviewed changes

experimental/Schemes/src/ToricBlowups/constructors.jl Show resolved Hide resolved

HereAround approved these changes Jan 16, 2025

View reviewed changes

benlorenz reviewed Jan 16, 2025

View reviewed changes

HereAround and others added 4 commits January 17, 2025 12:31

Update src/PolyhedralGeometry/PolyhedralFan/properties.jl

9645b32

Co-authored-by: Benjamin Lorenz <benlorenz@users.noreply.github.com>

Update test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl

d782694

Co-authored-by: Benjamin Lorenz <benlorenz@users.noreply.github.com>

Update test/AlgebraicGeometry/ToricVarieties/toric_blowups.jl

f82cc20

Co-authored-by: Benjamin Lorenz <benlorenz@users.noreply.github.com>

Update src/PolyhedralGeometry/PolyhedralFan/properties.jl

489204d

Co-authored-by: Benjamin Lorenz <benlorenz@users.noreply.github.com>

HereAround enabled auto-merge (squash) January 17, 2025 11:39

HereAround merged commit 269466a into oscar-system:master Jan 17, 2025
26 of 28 checks passed

paemurru deleted the ep/fix_blowup_along_singular_cone branch January 17, 2025 13:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix blowups along rays and singular cones #4454

Fix blowups along rays and singular cones #4454

paemurru commented Jan 11, 2025 •

edited

Loading

codecov bot commented Jan 11, 2025 •

edited

Loading

HereAround commented Jan 15, 2025 •

edited

Loading

paemurru commented Jan 15, 2025 •

edited

Loading

HereAround left a comment

benlorenz left a comment

Fix blowups along rays and singular cones #4454

Fix blowups along rays and singular cones #4454

Conversation

paemurru commented Jan 11, 2025 • edited Loading

codecov bot commented Jan 11, 2025 • edited Loading

Codecov Report

HereAround commented Jan 15, 2025 • edited Loading

paemurru commented Jan 15, 2025 • edited Loading

Bugs

Documentation

Correctness

HereAround left a comment

Choose a reason for hiding this comment

benlorenz left a comment

Choose a reason for hiding this comment

paemurru commented Jan 11, 2025 •

edited

Loading

codecov bot commented Jan 11, 2025 •

edited

Loading

HereAround commented Jan 15, 2025 •

edited

Loading

paemurru commented Jan 15, 2025 •

edited

Loading