-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add fast path for forwarddiff_color_jacobian! with sparse J #146
Add fast path for forwarddiff_color_jacobian! with sparse J #146
Conversation
Partial fix for #138 Bodges in a fast path for the case where J and sparsity are are both SparseMatrixCSC and have the same number of columns and stored values.
Codecov Report
@@ Coverage Diff @@
## master #146 +/- ##
==========================================
+ Coverage 83.05% 83.10% +0.05%
==========================================
Files 12 12
Lines 667 669 +2
==========================================
+ Hits 554 556 +2
Misses 113 113
Continue to review full report at Codecov.
|
@@ -316,3 +333,17 @@ function forwarddiff_color_jacobian!(J::AbstractMatrix{<:Number}, | |||
end | |||
return J | |||
end | |||
|
|||
# fast version of FiniteDiff._colorediteration! for the case where J and sparsity have the same sparsity pattern | |||
@inline function _colorediteration!(Jsparsity::SparseMatrixCSC,vfx,colorvec,color_i,ncols) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this just go to FiniteDiff.jl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK done... now requires PR JuliaDiff/FiniteDiff.jl#124 (however admit I don't fully understand how this could work and might be proliferating cut-and-paste cases instead of sane dispatches). Now we have:
|
Simplifies fast path logic. All other cases (J is not a SparseMatrixCSC, or J has a different sparsity pattern) use previous slow path.
Add a lower bound on 2.8.1 |
Partial fix for #138 #100
Add a fast path for the case where J and sparsity are are both SparseMatrixCSC
and have the same sparsity pattern.