-
-
Notifications
You must be signed in to change notification settings - Fork 214
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
Remove uses of NLsolve #2081
Remove uses of NLsolve #2081
Conversation
0a6fe51
to
f45e837
Compare
14d5263
to
668129a
Compare
failures seem to be same as master |
@@ -564,7 +563,7 @@ function _initialize_dae!(integrator, prob::DAEProblem, | |||
if alg.nlsolve !== nothing | |||
nlsolve = alg.nlsolve | |||
else | |||
nlsolve = NewtonRaphson(autodiff = isAD) | |||
nlsolve = NewtonRaphson(autodiff = alg_autodiff(integrator.alg)) |
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.
We should keep it as type information now that we can use it effectively.
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.
What does alg_autodiff
give? Can it give bool?
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.
Yes it changes the type level stuff to a bool as backwards compat support IIRC
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.
function alg_autodiff(alg)
autodiff = _alg_autodiff(alg)
if autodiff == Val(false)
return AutoFiniteDiff()
elseif autodiff == Val(true)
return AutoForwardDiff()
else
return _unwrap_val(autodiff)
end
end
This should constant propagate and give the correct AD type right?
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.
We shouldn't rely on constant propagation. It can fail. But let's pull this in and make this better in a follow up.
668129a
to
9226ed7
Compare
No description provided.