Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

matrix(QQ,foo) not working for vertices_and_rays output #3524

Open
YueRen opened this issue Mar 18, 2024 · 8 comments
Open

matrix(QQ,foo) not working for vertices_and_rays output #3524

YueRen opened this issue Mar 18, 2024 · 8 comments
Labels
bug Something isn't working topic: polyhedral geometry Issue concerns polyhedral geometry code

Comments

@YueRen
Copy link
Member

YueRen commented Mar 18, 2024

Apologies for the very crude report (busy with workshop), but the following errors were encountered with @yassineELMAAZOUZ

julia> VR = [0 0 0; 1 0 0; 0 1 0; -1 0 0];

julia> IM = IncidenceMatrix([[1,2,3],[1,3,4]]);

julia> far_vertices = [2,3,4];

julia> L = [0 0 1];

julia> PC = polyhedral_complex(IM, VR, far_vertices, L)
Polyhedral complex in ambient dimension 3

julia> matrix(QQ,vertices_and_rays(PC))
ERROR: ArgumentError: Matrix for Polymake not defined in this context.
Stacktrace:
 [1] matrix_for_polymake(iter::SubObjectIterator{Union{PointVector{QQFieldElem}, RayVector{QQFieldElem}}}; homogenized::Bool)
   @ Oscar ~/.julia/dev/Oscar/src/PolyhedralGeometry/iterators.jl:275
 [2] matrix_for_polymake
   @ ~/.julia/dev/Oscar/src/PolyhedralGeometry/iterators.jl:271 [inlined]
 [3] matrix(R::QQField, iter::SubObjectIterator{Union{PointVector{QQFieldElem}, RayVector{QQFieldElem}}})
   @ Oscar ~/.julia/dev/Oscar/src/PolyhedralGeometry/iterators.jl:292
 [4] top-level scope
   @ REPL[115]:1

@YueRen YueRen added the bug Something isn't working label Mar 18, 2024
@lgoettgens lgoettgens changed the title matrix(QQ,foo) not working matrix(QQ,foo) not working for vertices_and_rays output Mar 18, 2024
@YueRen YueRen added topic: polyhedral geometry Issue concerns polyhedral geometry code and removed topic: tropical geometry labels Mar 20, 2024
@YueRen
Copy link
Member Author

YueRen commented Mar 20, 2024

Thanks for fixing the issue title, @lgoettgens!

I've changed the labels, since this is more an issue in polyhedral geometry:

julia> VR = [0 0 0; 1 0 0; 0 1 0; -1 0 0];

julia> IM = IncidenceMatrix([[1,2,3],[1,3,4]]);

julia> far_vertices = [2,3,4];

julia> L = [0 0 1];

julia> PC = polyhedral_complex(IM, VR, far_vertices, L)
Polyhedral complex in ambient dimension 3

julia> matrix(QQ,vertices_and_rays(PC))
ERROR: ArgumentError: Matrix for Polymake not defined in this context.
Stacktrace:
 [1] matrix_for_polymake(iter::SubObjectIterator{Union{PointVector{QQFieldElem}, RayVector{QQFieldElem}}}; homogenized::Bool)
   @ Oscar ~/.julia/dev/Oscar/src/PolyhedralGeometry/iterators.jl:275
 [2] matrix_for_polymake
   @ ~/.julia/dev/Oscar/src/PolyhedralGeometry/iterators.jl:271 [inlined]
 [3] matrix(R::QQField, iter::SubObjectIterator{Union{PointVector{QQFieldElem}, RayVector{QQFieldElem}}})
   @ Oscar ~/.julia/dev/Oscar/src/PolyhedralGeometry/iterators.jl:292
 [4] top-level scope
   @ REPL[115]:1

@benlorenz
Copy link
Member

The issue with directly converting this to an Oscar matrix is somewhat related to what you complained about in the other issue, i.e. vectors losing type information. Merging the rays and vertices into one matrix might produce something like [1 0 0; 1 0 0] where the first line was a vertex and the second was a ray.
The other issue I see is that matrix(QQ, ...) only accepts Vector{<:Vector} but not Vector{<:AbstractVector}, so one workaround would be this:

julia> matrix(QQ, Vector.(vertices_and_rays(PC)))
[0   0]
[1   0]
[1   1]
[0   1]

In general I would really recommend not using vertices_and_rays but use vertices and rays separately. But we might need to fix something about the other constructor as well.
We will have another look at these constructors.

This issue is basically the same as #2313

@YueRen
Copy link
Member Author

YueRen commented Mar 20, 2024

@benlorenz Got it. When using vertices and rays (or minimal_faces and rays_modulo_lineality), is there a way to find out which ray or vertex the indices in the incidence matrix of the maximal polyhedra refer to?

julia> maximal_polyhedra(IncidenceMatrix,PC)
2×4 IncidenceMatrix
[1, 2, 3]
[1, 3, 4]

@benlorenz
Copy link
Member

You can use these instead:

julia> vertex_indices(maximal_polyhedra(PC))
2×3 IncidenceMatrix
[1, 2, 3]
[1, 3]


julia> ray_indices(maximal_polyhedra(PC))
2×1 IncidenceMatrix
[]
[1]

@YueRen
Copy link
Member Author

YueRen commented Mar 20, 2024

Oh wow, I didn't know these existed, thanks!

Is there by any chance also a constructor for PolyhedralComplex where you can enter vertices, rays, and their incidences separately? If yes, then I'm more than happy to forget about vertices_and_rays and from now on always work with vertices and rays seprately.

@benlorenz
Copy link
Member

benlorenz commented Mar 20, 2024

There is this one

function polyhedral_complex(f::scalar_type_or_field, 
                            v::AbstractCollection[PointVector],
                            vi::IncidenceMatrix, 
                            r::AbstractCollection[RayVector], 
                            ri::IncidenceMatrix,
                            L::Union{AbstractCollection[RayVector], Nothing} = nothing; 
                            non_redundant::Bool = false)

but it doesn't seem to work correctly with the SubobjectIterators right now, we will check it and add it to the docs.

Using matrices does work though:

julia> ppc = polyhedral_complex(QQ,
                                matrix(QQ,vertices(PC)),
                                vertex_indices(maximal_polyhedra(PC)),
                                matrix(QQ,rays(PC)),
                                ray_indices(maximal_polyhedra(PC)))
Polyhedral complex in ambient dimension 2

@fingolfin
Copy link
Member

@benlorenz wrote:

The other issue I see is that matrix(QQ, ...) only accepts Vector{<:Vector} but not Vector{<:AbstractVector},

Agreed, so "someone"(TM) should work on this

@fingolfin
Copy link
Member

@YueRen if this is important for you, please yell (or even better, fix it yourself ;-) but more seriously: just complain so we can prioritize it and assign someone to do it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working topic: polyhedral geometry Issue concerns polyhedral geometry code
Projects
None yet
Development

No branches or pull requests

4 participants