search

JuhaHeiskala / DirectQhull.jl

MIT License
11 stars 5 forks source link

Fix module name #1

Closed thchr closed 2 years ago

thchr commented 2 years ago

This fixes the problem I noted in https://github.com/JuliaPolyhedra/QHull.jl/issues/19#issuecomment-965974527.

thchr commented 2 years ago

Coincidentally, and somewhat unrelatedly, trying this on Windows, I hit:

julia> ConvexHull(rand(2,8))
ERROR: could not load symbol "qh_alloc_qh":
The specified procedure could not be found.
Stacktrace:
 [1] qh_alloc_qh (repeats 2 times)
   @ C:\Users\tchr\.julia\packages\DirectQhull\MDcrJ\src\DirectQhull.jl:63 [inlined]
 [2] ConvexHull(pnts::Matrix{Float64}, qhull_options::Vector{String}) (repeats 2 times)
   @ DirectQhull C:\Users\tchr\.julia\packages\DirectQhull\MDcrJ\src\DirectQhull.jl:345
 [3] top-level scope
   @ REPL[15]:1

which seems to fail on this line.

On Linux, 2D examples seem to work (nice!), but 3D fails with:

julia> ConvexHull(rand(3, 12))
ERROR: MethodError: no method matching iterate(::DirectQhull.var"#2#4"{Ptr{DirectQhull.qhT}})
Closest candidates are:
  iterate(::Union{LinRange, StepRangeLen}) at range.jl:664
  iterate(::Union{LinRange, StepRangeLen}, ::Int64) at range.jl:664
  iterate(::T) where T<:Union{Base.KeySet{var"#s79", var"#s78"} where {var"#s79", var"#s78"<:Dict}, Base.ValueIterator{var"#s77"} where var"#s77"<:Dict} at dict.jl:693
  ...
Stacktrace:
 [1] unique(itr::Function)
   @ Base ./set.jl:127
 [2] ConvexHull(pnts::Matrix{Float64}, qhull_options::Vector{String})
   @ DirectQhull ~/.julia/packages/DirectQhull/MDcrJ/src/DirectQhull.jl:364
 [3] ConvexHull(pnts::Matrix{Float64})
   @ DirectQhull ~/.julia/packages/DirectQhull/MDcrJ/src/DirectQhull.jl:345
 [4] top-level scope
   @ REPL[15]:1
JuhaHeiskala commented 2 years ago

This fixes the problem I noted in JuliaPolyhedra/QHull.jl#19 (comment).

Thanks, never crossed my mind to check the module name. :) I started writing this version from scratch and then made the mistake in module name.

JuhaHeiskala commented 2 years ago

For the windows error: julia> ConvexHull(rand(2,8)) ERROR: could not load symbol "qh_alloc_qh": This means that the Qhull_jll library does the have the qh_alloc_qh symbol. This is due to older version of Qhull_jll used, so updating Qhull_jll should fix it. I need to change the required version of Qhull_jll to the correct one in Project.toml, so the package manager will catch this.

JuhaHeiskala commented 2 years ago

The 3D error should also now be fixed. Implementation for 3D vertices had not been done correctly.

thchr commented 2 years ago

I checked whether the Windows error was still present: it is. From reading the related issue in Qhull.jl and the Yggdrasil PR to version bump of Qhull_jll, I had thought this might be fixed with v8.0.1000 of Qhull_jll - but it doesn't seem that way (just to confirm: I'm definitely on v8.0.1000 for Qhull_jll).

Is that expected?

JuhaHeiskala commented 2 years ago

Well, not expected. I do get the same error. The only reason I can think of is that the new accessor symbols are not defined in the qhull_r-exports.def and windows is adhering to the symbols explicitly exported in that file. Ping @stevengj

JuhaHeiskala commented 2 years ago

@thchr I made a PR to qhull to add the missing symbols, #105 Note also to @stevengj