Closed KnutAM closed 1 year ago
Merging #24 (63a4ef3) into main (fa53931) will decrease coverage by
2.31%
. The diff coverage is80.76%
.
:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.
@@ Coverage Diff @@
## main #24 +/- ##
==========================================
- Coverage 95.12% 92.81% -2.31%
==========================================
Files 7 7
Lines 164 181 +17
==========================================
+ Hits 156 168 +12
- Misses 8 13 +5
Impacted Files | Coverage Δ | |
---|---|---|
src/nlsolvers.jl | 89.23% <78.94%> (-5.00%) |
:arrow_down: |
src/QuasiStaticSolver.jl | 96.42% <85.71%> (-3.58%) |
:arrow_down: |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
@koehlerson, for the SteepestDescent
nlsolver, you expect the following update request, right?
update_problem!(problem, Δa; update_residual=true, update_jacobian=false)
Is there any case when you don't want this for the SteepestDescent
nlsolver, and I should make it optional like I've done for the Newton solver now?
I think this should always be true for SteepestDescent
solvers. I guess you would group solvers which do request a Jacobian update then into Quasi-Newton methods.
Edit: Sorry for the late reply!
Thanks!
Yes, but I think it is fine to keep the Newton
name, as it is pure Newton-Raphson with default arguments.
I'll update the docstring to highlight this!
SteepestDescent
solver(From FESolvers' perspective, this just implies setting the
update_jacobian
keyword argument toupdate_problem!
, it is the user that must branch to check what must be updated).