-
-
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
Standardize the Extension Algorithms #341
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #341 +/- ##
==========================================
- Coverage 88.04% 87.47% -0.57%
==========================================
Files 27 28 +1
Lines 2174 2172 -2
==========================================
- Hits 1914 1900 -14
- Misses 260 272 +12 ☔ View full report in Codecov by Sentry. |
2779c92
to
c79d228
Compare
c79d228
to
f2edda0
Compare
function (u) | ||
du = similar(u) | ||
prob.f(du, u, prob.p) | ||
return du | ||
end |
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.
This shouldn't originate in the first place.
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.
Yeah, the scalar case root finding doesn't need this
e987f91
to
e3ad1d1
Compare
d3131c9
to
fd08605
Compare
fd08605
to
2d19f54
Compare
@@ -9,7 +9,7 @@ isinplace(JᵀJ::KrylovJᵀJ) = isinplace(JᵀJ.Jᵀ) | |||
|
|||
# Select if we are going to use sparse differentiation or not | |||
sparsity_detection_alg(_, _) = NoSparsityDetection() | |||
function sparsity_detection_alg(f, ad::AbstractSparseADType) | |||
function sparsity_detection_alg(f::NonlinearFunction, ad::AbstractSparseADType) |
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.
Does this need to specialize?
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.
safety measure, we are using f.sparsity
and stuff in there.
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.
I didn't go through all the details but it seems generally in the right direction with all of it.
Are the 23 test problem failures known? |
I presume that's what was fixed in the problem library, so it should have passed? |
Checklist
jacobian!!
in FastLM