JuhaHeiskala / DirectQhull.jl

MIT License
12 stars 5 forks source link

Add an auto-freeing interface when allocating pointers to qhull #9

Closed dgleich closed 1 year ago

dgleich commented 1 year ago

Please let me know if you think we should approach this a different way. Currently, this just allocates a Qhull structure, works with the external interfaces, and then frees it and all associated memory. This mirrors the "open(...) do f; end" way that files are often used.

On the other hand, this will be wrong if any memory is mirrored in the Julia structures from Qhull.

This interface has the advantage that is works better with multithreaded code as the finalizer mechanism to free memory can be troublesome in multithreaded scenarios.

dgleich commented 1 year ago

I have not done extensive testing on it. So if you like the interface, I'll do more testing to make sure it doesn't break it subtle ways.

JuhaHeiskala commented 1 year ago

All data is not loaded into Julia's memory from inside qhull in the various constructors (ConvexHull and the rest), i.e. those that are accessed through getpropertycalls. So using those fields/properties would not then work. This may or may not be an issue. Automatically releasing the memory is of course nice. So currently it is possible to e.g. traverse the qhull internal structures after the construction, but I don't know if this ability is really needed. @thchr any comment on this? I don't myself work in this area, so really can't say. (I did the original version of DirectQhull for a very specific purpose)

Maybe the automatic release of qhull's memory should at least be an option in the constructors? With default value one way or the other.

dgleich commented 1 year ago

I meant all data in the Julia struct that is returned. A lot of this is copied into native Julia arrays/memory. I realize there are things one might want to do with the qh_ptr that aren't in these.

The way I always look at these things is that layered / scalable interfaces are useful. There should be a simple interface that just handles everything as simply as possible. There may be tradeoffs that balance towards simplicity vs. every conceivable use-case. e.g. many use-cases will point down to just getting the info for a Voronoi or Delaunay or ConvexHull for a set of points, so these ought to be easy and simple. And the current interface makes largely gives this. (i.e. there are some other helper functions that might be nice to add for things like Voronoi regions to handle projecting to a finite bounding box.)

Then there should also be expert interfaces that enable more sophisticated users to build anything they might want from the underlying library.

So it just depends on what you'd like DirectQhull to be :)

It's okay to leave DirectQhull.jl as just a set of expert user interfaces! (e.g. calls to the Qhull library.)

So the proposal I would make is: (This pull request is a start to this bigger interface...)

So I definitely don't want to take away options that allow one to get into the weeds with the qhull library. Just want to make the common use cases into easy function calls.