mlubin / ReverseDiffSparse.jl

Reverse-mode automatic differentiation for sparse Hessians
Other
24 stars 12 forks source link

unsafe workaround for slow subarrays #4

Closed mlubin closed 10 years ago

mlubin commented 10 years ago

Gives a 3.3x speedup on a model with 1,000,000 constraints, which means that we create 1,000,000 array views on every function call. Any allocations in this loop are bad news given the quadratic performance of the GC. @timholy, do you know of any safer way to make a zero-allocation array view object? @IainNZ @joehuchette

coveralls commented 10 years ago

Coverage Status

Coverage increased (+0.0%) when pulling ac46fce0ec6158701ffb5187e04ab8f1f37ccadf on fast_view into 78c81cb0aa813def60a6556c3d936362c5d25811 on master.

IainNZ commented 10 years ago

Ha @mlubin you mad man. I like it. I mean, its unsafe in some sense, but its user facing so we know its always correct.

timholy commented 10 years ago

Two questions:

You might be able to mitigate some issues by storing a reference to the array (rather than a pointer). But I confess I've used similar tricks in some places in my own code.

mlubin commented 10 years ago

Will this only be used on Arrays? pointer may not be implemented for an arbitrary AbstractArray type.

I'll probably settle on DenseVector.

Presumably you're aware that you need to maintain a reference to the underlying array, to keep it from being GCed (passing a VectorView does not count as a reference).

The vector view isn't stored anywhere, so the original vector will always have a longer lifespan.

I tried storing a reference to the original array, but that seemed to trigger extra allocations.

coveralls commented 10 years ago

Coverage Status

Coverage increased (+0.0%) when pulling 8e3ade867581aad6ade7c898ada2ed58e0ad42bb on fast_view into 78c81cb0aa813def60a6556c3d936362c5d25811 on master.

timholy commented 10 years ago

I tried storing a reference to the original array, but that seemed to trigger extra allocations.

I think when I tried something like this, I found the same thing. So this seems like a reasonable solution.

mlubin commented 10 years ago

Cool, thanks for the feedback @timholy