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

Fix module name #1

Merged
merged 1 commit into from
Nov 13, 2021
Merged

Fix module name #1

merged 1 commit into from
Nov 13, 2021

Conversation

thchr
Copy link
Collaborator

@thchr thchr commented Nov 12, 2021

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

@thchr
Copy link
Collaborator Author

thchr commented Nov 12, 2021

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
Copy link
Owner

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 JuhaHeiskala merged commit b244cd1 into JuhaHeiskala:master Nov 13, 2021
@JuhaHeiskala
Copy link
Owner

JuhaHeiskala commented Nov 13, 2021

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
Copy link
Owner

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

@thchr
Copy link
Collaborator Author

thchr commented Nov 16, 2021

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
Copy link
Owner

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
Copy link
Owner

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

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

Successfully merging this pull request may close these issues.

2 participants