ToricVarieties: Toric morphisms #1460

HereAround · 2022-07-14T19:10:32Z

This PR achieves the following:

Implement basic functionality for toric morphisms.
For a given toric morphism, it computes the induced maps on the torusinvariant Weil divisors and the Cartier divisors.
It implements the Cox variety and the quotient morphism from the Cox variety to the toric variety in question.

src/ToricVarieties/ToricMorphisms/attributes.jl

HereAround · 2022-10-26T19:51:50Z

I just rebased to the latest master.

lkastner

Please avoid high nesting levels and long functions. If funtions become too long they should probably be broken up into shorter logical parts.

lkastner · 2022-10-27T09:24:32Z

src/ToricVarieties/ToricMorphisms/attributes.jl

+    for i in 1:nrows(images_of_rays)
+      for j in 1:length(codomain_variety_max_cones)
+        if contains(codomain_variety_max_cones[j], [images_of_rays[i,k] for k in 1:ncols(images_of_rays)])
+          rayimage_maxcones_incidence_list[i] = j
+          break
+        end
+      end
+    end


This is duplicating a lot of checks, isn't it? Also I believe Max said that there is a way to avoid nested for loops, I think you can just separate the two params with a comma.

Nevermind, I was just confused. Can you not write something like images_of_rays[i,:]?

Also what if a ray is contained in several maximal cones?

lkastner · 2022-10-27T09:41:53Z

src/ToricVarieties/ToricMorphisms/attributes.jl

+    images_of_rays = domain_variety_rays * matrix(grid_morphism(tm))
+
+    # for each image ray, find a max cone in the codomain toric variety which contains it
+    codomain_variety_max_cones = maximal_cones(codomain_variety)
+    rayimage_maxcones_incidence_list = [0 for i in 1:nrows(images_of_rays)]
+    for i in 1:nrows(images_of_rays)
+      for j in 1:length(codomain_variety_max_cones)
+        if contains(codomain_variety_max_cones[j], [images_of_rays[i,k] for k in 1:ncols(images_of_rays)])
+          rayimage_maxcones_incidence_list[i] = j
+          break
+        end
+      end
+    end
+
+    # form list of rays which are in maximal cones of the codomain variety
+    codomain_variety_maxcone_ray_incidence = ray_indices(maximal_cones(codomain_variety))
+    r = rank(character_lattice(codomain_variety))
+    codomain_variety_rays_in_cones = []
+    for i in 1:length(codomain_variety_max_cones)
+        matrix = zeros(Int,nrows(codomain_variety_rays),ncols(codomain_variety_rays))
+        for j in 1:nrows(codomain_variety_rays)
+          if codomain_variety_maxcone_ray_incidence[i,j]
+            for k in 1:ncols(codomain_variety_rays)
+              matrix[j,k] = codomain_variety_rays[j,k]
+            end
+          end
+        end
+        push!(codomain_variety_rays_in_cones, matrix)
+    end


You can actually use IncidenceMatrix from Oscar and build an incidence matrix for the image rays in the target fan in hopefully less code along the lines of

julia> fc = normal_fan(cube(2)); julia> fh = normal_fan(hexagon); julia> rh = collect(rays(fh)) 6-element Vector{RayVector{fmpq}}: [1, 0] [0, 1] [1, -1] [-1, 1] [-1, 0] [0, -1] julia> mc = collect(maximal_cones(fc)) 4-element Vector{Cone{fmpq}}: A polyhedral cone in ambient dimension 2 A polyhedral cone in ambient dimension 2 A polyhedral cone in ambient dimension 2 A polyhedral cone in ambient dimension 2 julia> image_incidences = IncidenceMatrix(nrays(fh), n_maximal_cones(fc)) 6×4 IncidenceMatrix [] [] [] [] [] [] julia> for i in 1:nrays(fh), j in 1:n_maximal_cones(fc) image_incidences[i,j] = contains(mc[j], rh[i]) end julia> image_incidences 6×4 IncidenceMatrix [1, 3] [1, 2] [3] [2] [2, 4] [3, 4]

Does this maybe simplify this method?

lkastner · 2022-10-27T09:42:48Z

src/ToricVarieties/ToricMorphisms/attributes.jl

+      end
+    end
+
+    # return the map


Suggested change

# return the map

Isn't this obvious from the line below starting with return?

lkastner · 2022-10-27T09:49:09Z

src/ToricVarieties/ToricMorphisms/attributes.jl

+    for i in 1:nrows(images_of_rays)
+      current_row = images_of_rays[i,:]
+      solution = false
+      matrix_multiplier = 1
+      while !solution
+        r = matrix_multiplier * current_row
+        matrix_multiplier = matrix_multiplier + 1
+        m = matrix(ZZ,codomain_variety_rays_in_cones[rayimage_maxcones_incidence_list[i]])
+        solution = can_solve(transpose(m), transpose(r))
+        if solution
+          current_row = solve(transpose(m), transpose(r))
+          current_row = transpose(current_row)
+        end
+      end


Why not while !can_solve(...)? Also the matrix_multiplier seems unneeded. You could just add images_of_rays[i,:] to current_row and addition might be faster than multiplication.

HereAround · 2022-10-28T21:53:55Z

Thank you @lkastner for the detailed review. Based on your feedback, I believe there is a need to rethink and also redesign a couple of things here, in particular the function for the map between the torus invariant divisor class groups. Hence, I have reverted this PR into a draft for the time being. More shortly.

…phism

HereAround · 2022-10-31T18:10:38Z

Thank you @lkastner. I merge.

HereAround marked this pull request as draft July 14, 2022 19:10

HereAround added the topic: toric varieties label Jul 14, 2022

HereAround mentioned this pull request Jul 14, 2022

Toric Varieties: Completed goals from fall 2021 until December 2022 #763

Closed

32 tasks

HereAround added the enhancement New feature or request label Jul 15, 2022

HereAround force-pushed the ToricMorphisms branch 4 times, most recently from 5bf931f to 4d3891f Compare July 15, 2022 12:15

fingolfin reviewed Jul 18, 2022

View reviewed changes

src/ToricVarieties/ToricMorphisms/attributes.jl Outdated Show resolved Hide resolved

HereAround force-pushed the ToricMorphisms branch 5 times, most recently from 68cdbe3 to beade5c Compare July 21, 2022 10:20

HereAround force-pushed the ToricMorphisms branch 3 times, most recently from 2d15000 to 2480c5a Compare September 1, 2022 03:18

HereAround force-pushed the ToricMorphisms branch from 2480c5a to 1ffae7e Compare October 18, 2022 13:51

HereAround mentioned this pull request Oct 19, 2022

Toric covered schemes #1636

Merged

HereAround force-pushed the ToricMorphisms branch 8 times, most recently from 46df69e to 4b1e10a Compare October 25, 2022 11:46

HereAround marked this pull request as ready for review October 25, 2022 11:49

HereAround requested a review from lkastner October 25, 2022 11:49

HereAround force-pushed the ToricMorphisms branch from 4b1e10a to ea24ea3 Compare October 25, 2022 13:53

HereAround mentioned this pull request Oct 25, 2022

ToricVarieties: Further improvements #1650

Merged

HereAround requested a review from fingolfin October 25, 2022 18:07

HereAround changed the title ~~ToricVarieties: Toric morphisms (Draft)~~ ToricVarieties: Toric morphisms Oct 25, 2022

HereAround force-pushed the ToricMorphisms branch from ea24ea3 to 47e5d89 Compare October 26, 2022 19:51

lkastner requested changes Oct 27, 2022

View reviewed changes

HereAround marked this pull request as draft October 28, 2022 21:51

HereAround added 3 commits October 31, 2022 13:14

ToricVarieties: Begin support for toric morphisms

43d58f4

ToricVarieties: Add map of torusinvariant_weil_divisors for toric mor…

613dc1c

…phism

ToricVarieites: Implement Cox variety

1777389

HereAround force-pushed the ToricMorphisms branch from 47e5d89 to 1777389 Compare October 31, 2022 15:52

lkastner approved these changes Oct 31, 2022

View reviewed changes

HereAround marked this pull request as ready for review October 31, 2022 18:10

HereAround merged commit 1c0fce2 into oscar-system:master Oct 31, 2022

HereAround deleted the ToricMorphisms branch October 31, 2022 18:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ToricVarieties: Toric morphisms #1460

ToricVarieties: Toric morphisms #1460

HereAround commented Jul 14, 2022 •

edited

Loading

HereAround commented Oct 26, 2022

lkastner left a comment

lkastner Oct 27, 2022

lkastner Oct 27, 2022

lkastner Oct 27, 2022

lkastner Oct 27, 2022

lkastner Oct 27, 2022

lkastner Oct 27, 2022

HereAround commented Oct 28, 2022

HereAround commented Oct 31, 2022

ToricVarieties: Toric morphisms #1460

ToricVarieties: Toric morphisms #1460

Conversation

HereAround commented Jul 14, 2022 • edited Loading

HereAround commented Oct 26, 2022

lkastner left a comment

Choose a reason for hiding this comment

lkastner Oct 27, 2022

Choose a reason for hiding this comment

lkastner Oct 27, 2022

Choose a reason for hiding this comment

lkastner Oct 27, 2022

Choose a reason for hiding this comment

lkastner Oct 27, 2022

Choose a reason for hiding this comment

lkastner Oct 27, 2022

Choose a reason for hiding this comment

lkastner Oct 27, 2022

Choose a reason for hiding this comment

HereAround commented Oct 28, 2022

HereAround commented Oct 31, 2022

HereAround commented Jul 14, 2022 •

edited

Loading