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

3D volume fails on tiny point sets #321

Open
bqi343 opened this issue Mar 26, 2023 · 6 comments
Open

3D volume fails on tiny point sets #321

bqi343 opened this issue Mar 26, 2023 · 6 comments

Comments

@bqi343
Copy link

bqi343 commented Mar 26, 2023

When using the default library (or CDDLib), an AssertionError is thrown when computing the volume of a 3D polyhedron where the same point is passed twice to vrep.

using Polyhedra, CDDLib, QHull

const libs = [Polyhedra.default_library(3, Float64), CDDLib.Library(:float), QHull.Library()]

function get_volume(point_set::Matrix{T}, lib) where {T}
    poly = polyhedron(vrep(point_set), lib)
    Polyhedra.volume(poly)
end

@testset "test_volume_tricky" begin
    points = [-1.0 0.0 1.0
        0.0 0.0 1.0
        0.0 0.0 0.0
        -1.0 -1.0 0.0
        -1.0 -1.0 0.0]
    for lib in libs
        # same error for default library or CDDLib
        # expected answer (0.16666666666666666) for qhull
        println(lib, " ", get_volume(points, lib))
    end
end
Full Stacktrace
test_volume_tricky: Error During Test at /Users/benq/.julia/packages/ReTest/WnRIG/src/ReTest.jl:517
  Got exception outside of a @test
  AssertionError: codim >= 0
  Stacktrace:
    [1] _triangulation(Δs::Vector{Vector{Polyhedra.Index{Float64, Vector{Float64}}}}, Δ::Vector{Polyhedra.Index{Float64, Vector{Float64}}}, v_idx::Vector{Polyhedra.Index{Float64, Vector{Float64}}}, h_idx::Vector{Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}}, incident_idx::Dict{Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}, Set{Polyhedra.Index{Float64, Vector{Float64}}}}, is_weak_adjacent::Dict{Tuple{Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}, Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}}, Bool}, codim::Int64)
      @ Polyhedra ~/.julia/packages/Polyhedra/Xhcfx/src/triangulation.jl:10
    [2] _triangulation(Δs::Vector{Vector{Polyhedra.Index{Float64, Vector{Float64}}}}, Δ::Vector{Polyhedra.Index{Float64, Vector{Float64}}}, v_idx::Vector{Polyhedra.Index{Float64, Vector{Float64}}}, h_idx::Vector{Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}}, incident_idx::Dict{Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}, Set{Polyhedra.Index{Float64, Vector{Float64}}}}, is_weak_adjacent::Dict{Tuple{Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}, Polyhedra.Index{Float64, HalfSpace{Float64, Vector{Float64}}}}, Bool}, codim::Int64) (repeats 4 times)
      @ Polyhedra ~/.julia/packages/Polyhedra/Xhcfx/src/triangulation.jl:34
    [3] triangulation_indices(p::DefaultPolyhedron{Float64, MixedMatHRep{Float64, Matrix{Float64}}, MixedMatVRep{Float64, Matrix{Float64}}})
      @ Polyhedra ~/.julia/packages/Polyhedra/Xhcfx/src/triangulation.jl:46
    [4] triangulation
      @ ~/.julia/packages/Polyhedra/Xhcfx/src/triangulation.jl:50 [inlined]
    [5] volume(p::DefaultPolyhedron{Float64, MixedMatHRep{Float64, Matrix{Float64}}, MixedMatVRep{Float64, Matrix{Float64}}})
      @ Polyhedra ~/.julia/packages/Polyhedra/Xhcfx/src/polyhedron.jl:100
    [6] get_volume(point_set::Matrix{Float64}, lib::DefaultLibrary{Float64})
      @ Main.InnerProductMaxTests ~/Dev/InnerProductMax/test/compare_chull.jl:9
    [7] macro expansion
      @ ~/Dev/InnerProductMax/test/compare_chull.jl:15 [inlined]
    [8] macro expansion
      @ ~/.julia/packages/ReTest/WnRIG/src/testset.jl:654 [inlined]
    [9] macro expansion
      @ ~/Dev/InnerProductMax/test/compare_chull.jl:13 [inlined]
   [10] top-level scope
      @ ~/.julia/packages/ReTest/WnRIG/src/ReTest.jl:517
   [11] eval
      @ ./boot.jl:368 [inlined]
   [12] (::ReTest.var"#80#98"{ReTest.Testset.Format})(mod::Module, ts::ReTest.TestsetExpr, pat::ReTest.And, chan::NamedTuple{(:out, :compute, :preview), Tuple{Distributed.RemoteChannel{Channel{Union{Nothing, ReTest.Testset.ReTestSet}}}, Channel{Nothing}, Nothing}})
      @ ReTest ~/.julia/packages/ReTest/WnRIG/src/ReTest.jl:1160
   [13] #invokelatest#2
      @ ./essentials.jl:729 [inlined]
   [14] invokelatest
      @ ./essentials.jl:726 [inlined]
   [15] #153
      @ /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Distributed/src/remotecall.jl:425 [inlined]
   [16] run_work_thunk(thunk::Distributed.var"#153#154"{ReTest.var"#80#98"{ReTest.Testset.Format}, Tuple{Module, ReTest.TestsetExpr, ReTest.And, NamedTuple{(:out, :compute, :preview), Tuple{Distributed.RemoteChannel{Channel{Union{Nothing, ReTest.Testset.ReTestSet}}}, Channel{Nothing}, Nothing}}}, Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}}}, print_error::Bool)
      @ Distributed /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Distributed/src/process_messages.jl:70
   [17] remotecall_fetch(::Function, ::Distributed.LocalProcess, ::Module, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Distributed /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Distributed/src/remotecall.jl:450
   [18] remotecall_fetch(::Function, ::Distributed.LocalProcess, ::Module, ::Vararg{Any})
      @ Distributed /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Distributed/src/remotecall.jl:449
   [19] remotecall_fetch(::Function, ::Int64, ::Module, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ Distributed /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Distributed/src/remotecall.jl:492
   [20] remotecall_fetch
      @ /Applications/Julia-1.8.app/Contents/Resources/julia/share/julia/stdlib/v1.8/Distributed/src/remotecall.jl:492 [inlined]
   [21] macro expansion
      @ ~/.julia/packages/ReTest/WnRIG/src/ReTest.jl:1157 [inlined]
   [22] (::ReTest.var"#78#96"{Int64, ReTest.Testset.Format, Nothing, ReTest.Testset.ReTestSet, Base.Threads.Atomic{Bool}, Channel{Nothing}, Distributed.RemoteChannel{Channel{Union{Nothing, ReTest.Testset.ReTestSet}}}, Vector{Any}, ReTest.And, Module})()
      @ ReTest ./task.jl:484
@bqi343 bqi343 changed the title 3D volume fails on duplicate points 3D volume fails on tiny point sets Mar 26, 2023
@bqi343
Copy link
Author

bqi343 commented Mar 26, 2023

There are also many instances where no error is thrown but both the default library and cddlib give twice the volume of that returned by qhull.

@testset "test_volume_tricky3" begin
    points = [-1.0 1.0 1.0
        -1.0 -1.0 1.0
        1.0 0.0 -1.0
        1.0 1.0 1.0
        0.0 1.0 -1.0]
    for lib in libs # default -> 5.333333333333333, qhull -> 2.6666666666666665 (qhull is correct)
        vol = get_volume(points, lib, false)
        @test vol  8.0 / 3
        if !(vol  8.0 / 3)
            println("failed $lib got $vol")
        end
    end
end

@apaoluzzi
Copy link

apaoluzzi commented Mar 27, 2023 via email

@bqi343
Copy link
Author

bqi343 commented Mar 27, 2023

I don't see any included visualization, and I'm pretty sure 8/4 is wrong considering that

  1. QHull agrees with 8/3, and presumably (?) it's been tested extensively
import numpy as np
from scipy.spatial import ConvexHull

points = np.array([[-1.0, 1.0, 1.0],
    [-1.0, -1.0, 1.0],
    [1.0, 0.0, -1.0],
    [1.0, 1.0, 1.0],
    [0.0, 1.0, -1.0]])

print(ConvexHull(points).volume) # 2.6666666666666665
  1. From manual inspection, the hull splits into the tetrahedrons labeled by $(1, 2, 4, 5)$ and $(2, 3, 4, 5)$, both of which have a volume of $8/6$, for a total of $8/3$.
using Polyhedra: polyhedron, vrep, volume
using LinearAlgebra

points = [-1.0 1.0 1.0
    -1.0 -1.0 1.0
    1.0 0.0 -1.0
    1.0 1.0 1.0
    0.0 1.0 -1.0]

function volume_simplex(points)
    A = Matrix{T}(undef, 3, 3)
    for i in 1:3
        A[i, :] = points[i+1, :] - points[1, :]
    end
    return abs(det(A)) / 6
end

p = polyhedron(vrep(points))
println(volume(p)) # 5.3..., wrong
vol_1 = volume_simplex(points[[1, 2, 4, 5], :])
vol_2 = volume_simplex(points[[2, 4, 5, 3], :])
println("$vol_1 $vol_2 $(vol_1 + vol_2)") # 1.3..., 1.3..., 2.6..., as expected

Screen Shot 2023-03-27 at 10 11 31 AM

@apaoluzzi
Copy link

apaoluzzi commented Mar 27, 2023 via email

@bqi343
Copy link
Author

bqi343 commented Mar 27, 2023

I didn't run your code, but it looks like the last two faces in TW should be 2,1,5 and 2,5,3.

@apaoluzzi
Copy link

apaoluzzi commented Mar 27, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants