search

JuliaDiff / SparseDiffTools.jl

Fast jacobian computation through sparsity exploitation and matrix coloring
MIT License
237 stars 41 forks source link

Fix type stability and allow JacVec to be non-square jacobians #274

Closed avik-pal closed 7 months ago

codecov[bot] commented 7 months ago

Codecov Report

Attention: 17 lines in your changes are missing coverage. Please review.

Comparison is base (9a713c6) 86.62% compared to head (84c0dc3) 84.72%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #274 +/- ## ========================================== - Coverage 86.62% 84.72% -1.91% ========================================== Files 21 22 +1 Lines 1286 1283 -3 ========================================== - Hits 1114 1087 -27 - Misses 172 196 +24 ``` | [Files](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff) | Coverage Δ | | |---|---|---| | [src/SparseDiffTools.jl](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL1NwYXJzZURpZmZUb29scy5qbA==) | `100.00% <ø> (ø)` | | | [src/differentiation/jaches\_products.jl](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL2RpZmZlcmVudGlhdGlvbi9qYWNoZXNfcHJvZHVjdHMuamw=) | `93.78% <100.00%> (-1.75%)` | :arrow_down: | | [src/differentiation/vecjac\_products.jl](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL2RpZmZlcmVudGlhdGlvbi92ZWNqYWNfcHJvZHVjdHMuamw=) | `89.28% <100.00%> (+8.47%)` | :arrow_up: | | [src/differentiation/common.jl](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/274?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff#diff-c3JjL2RpZmZlcmVudGlhdGlvbi9jb21tb24uamw=) | `59.52% <59.52%> (ø)` | | ... and [7 files with indirect coverage changes](https://app.codecov.io/gh/JuliaDiff/SparseDiffTools.jl/pull/274/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=JuliaDiff)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

avik-pal commented 7 months ago

@ChrisRackauckas the failure is because, we did not throw a warning before. The main change here is that if someone passes p and t, the general expectation should be that the function also accepts those as arguments. In the previous implementation, we dropped p and t after taking them as inputs, so I am giving a warning that the input function doesn't accept them as inputs

ChrisRackauckas commented 7 months ago

That's fine, we should just make sure we fix downstream before merging so that way it's ready.