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

solve_mixed should check arguments for consistency #1678

Closed
HechtiDerLachs opened this issue Nov 2, 2022 · 8 comments · Fixed by #1679
Closed

solve_mixed should check arguments for consistency #1678

HechtiDerLachs opened this issue Nov 2, 2022 · 8 comments · Fixed by #1679
Assignees
Labels
bug Something isn't working

Comments

@HechtiDerLachs
Copy link
Collaborator

Try the following code:

A = ZZ[2 1 0;-1 -1 -1]
v = ZZ[2; -1]
Id = identity_matrix(ZZ, ncols(A))
solve_mixed(A, v, Id, zero(v))

According to the documentation of solve_mixed, this should look for solutions of A*x = v under the condition that x >= 0. The obvious solution is x = [1; 0; 0], but it throws an error.

Interestingly, the code runs when we replace A by A = ZZ[2 1; -1 -1].

CC: @lkastner @benlorenz

@HechtiDerLachs HechtiDerLachs added the bug Something isn't working label Nov 2, 2022
@thofma
Copy link
Collaborator

thofma commented Nov 2, 2022

What is the error?

@HechtiDerLachs
Copy link
Collaborator Author

Yes, sorry. I get the following message:

ERROR: LoadError: DimensionMismatch("tried to assign 2-element array to 3×1 destination")
Stacktrace:
  [1] throw_setindex_mismatch(X::Vector{BigInt}, I::Tuple{Int64, Int64})
    @ Base ./indices.jl:193
  [2] setindex_shape_check
    @ ./indices.jl:248 [inlined]
  [3] _unsafe_setindex!(::IndexLinear, ::Matrix{BigInt}, ::Vector{BigInt}, ::Base.Slice{Base.OneTo{Int64}}, ::Int64)
    @ Base ./multidimensional.jl:896
  [4] _setindex!
    @ ./multidimensional.jl:887 [inlined]
  [5] setindex!
    @ ./abstractarray.jl:1267 [inlined]
  [6] augment(mat::Matrix{BigInt}, vec::Vector{BigInt})
    @ Oscar ~/Kaiserslautern/neuer_Oscar_Klon/Oscar.jl/src/PolyhedralGeometry/helpers.jl:158
  [7] affine_matrix_for_polymake(x::Tuple{Matrix{BigInt}, Vector{BigInt}})
    @ Oscar ~/Kaiserslautern/neuer_Oscar_Klon/Oscar.jl/src/PolyhedralGeometry/helpers.jl:35
  [8] Polyhedron{fmpq}(I::Tuple{Matrix{BigInt}, Vector{BigInt}}, E::Tuple{Matrix{BigInt}, Vector{BigInt}})
    @ Oscar ~/Kaiserslautern/neuer_Oscar_Klon/Oscar.jl/src/PolyhedralGeometry/Polyhedron/constructors.jl:101
  [9] Polyhedron
    @ ~/Kaiserslautern/neuer_Oscar_Klon/Oscar.jl/src/PolyhedralGeometry/Polyhedron/constructors.jl:23 [inlined]
 [10] solve_mixed(A::fmpz_mat, b::fmpz_mat, C::fmpz_mat, d::fmpz_mat)
    @ Oscar ~/Kaiserslautern/neuer_Oscar_Klon/Oscar.jl/src/PolyhedralGeometry/solving_integrally.jl:32
 [11] top-level scope
    @ /tmp/test.jl:9
 [12] include(fname::String)
    @ Base.MainInclude ./client.jl:444
 [13] top-level scope
    @ REPL[25]:1
 [14] top-level scope
    @ ~/.julia/packages/Infiltrator/r3Hf5/src/Infiltrator.jl:710

@thofma
Copy link
Collaborator

thofma commented Nov 2, 2022

Probably some index error. In the meantime you could do

julia> solve_mixed(A, v, Id)
[1   0   0]`

(Assuming that you only have >= 0.)

@HechtiDerLachs
Copy link
Collaborator Author

The shortened version works? This part of code was suggested to me by @lkastner once and I just kept with it. But I can switch to the short one. However, the issue should eventually be resolved.

@fingolfin
Copy link
Member

Your final vector in the argument list has the wrong length.

@HechtiDerLachs
Copy link
Collaborator Author

Now I really wish this was not true... But it probably is. Yes, sorry, I just didn't bother checking this again. I only remember this line being suggested to me by @lkastner once upon a time. But it had already been late in the evening at that point and probably something got lost in translation.

Either way, a plausibility check and a more informative error message would probably be helpful.

Thanks for catching my mistake and my apologies for the mess!

@thofma thofma reopened this Nov 2, 2022
@thofma
Copy link
Collaborator

thofma commented Nov 2, 2022

We should check the input for consistency.

@thofma thofma changed the title solve_mixed does not seem to do its job properly solve_mixed should check arguments for consistency Nov 2, 2022
@fingolfin
Copy link
Member

I agree a more helpful error message would be good!

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

Successfully merging a pull request may close this issue.

5 participants