NonlinearSolve.jl icon indicating copy to clipboard operation
NonlinearSolve.jl copied to clipboard

Handle downstream usage issues

Open ChrisRackauckas opened this issue 3 years ago • 2 comments

It seems this library wasn't properly tested...

ChrisRackauckas avatar Sep 15 '22 17:09 ChrisRackauckas

Codecov Report

Merging #88 (a9a82b6) into master (6347ae5) will increase coverage by 37.22%. The diff coverage is 58.33%.

@@             Coverage Diff             @@
##           master      #88       +/-   ##
===========================================
+ Coverage    0.00%   37.22%   +37.22%     
===========================================
  Files           8        5        -3     
  Lines         459      231      -228     
===========================================
+ Hits            0       86       +86     
+ Misses        459      145      -314     
Impacted Files Coverage Δ
src/utils.jl 12.38% <12.90%> (+12.38%) :arrow_up:
src/raphson.jl 50.00% <60.86%> (+50.00%) :arrow_up:
src/NonlinearSolve.jl 100.00% <100.00%> (ø)
src/ad.jl 100.00% <100.00%> (ø)
src/jacobian.jl 43.24% <100.00%> (+43.24%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov[bot] avatar Sep 15 '22 17:09 codecov[bot]

The tests here are not running at all. The code coverage has been zero for more than half a year https://app.codecov.io/gh/SciML/NonlinearSolve.jl. The reason is that you only run the tests for https://github.com/SciML/NonlinearSolve.jl/blob/22ed463491d2dd86758d90e79e297943e0b5914d/test/runtests.jl#L10 but GROUP never has any of those values https://github.com/SciML/NonlinearSolve.jl/blob/22ed463491d2dd86758d90e79e297943e0b5914d/.github/workflows/CI.yml#L15-L16. Might be good to add an else branch that errors.

andreasnoack avatar Sep 15 '22 19:09 andreasnoack

Finally.

ChrisRackauckas avatar Nov 24 '22 09:11 ChrisRackauckas

Can the downstream failure safely be ignored?

andreasnoack avatar Nov 24 '22 09:11 andreasnoack

Yes, this is getting a v1.0 release, and MTK is moving to SimpleNonlinerSolve.jl. It took awhile because this library had to be split to get the dependencies correct, which is a very breaking change. It also unifies the arguments (abstol instead of tol), so it's a very breaking update. Basically, it's finally updated from being an overlooked intern project to a full compliant piece of SciML, with all of the breakage required to make it conform to the rules of the interface. The update should be relatively sane though since most things would actually just move to SimpleNonlinearSolve.jl: the "bigger" Newton here is really just for big systems which isn't used all that much downstream.

ChrisRackauckas avatar Nov 24 '22 09:11 ChrisRackauckas