Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 3 commits into from
Nov 15, 2023
Merged

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

merged 3 commits into from
Nov 15, 2023

Conversation

avik-pal
Copy link
Contributor

No description provided.

Copy link

codecov bot commented Nov 13, 2023

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
@@            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 Coverage Δ
src/SparseDiffTools.jl 100.00% <ø> (ø)
src/differentiation/jaches_products.jl 93.78% <100.00%> (-1.75%) ⬇️
src/differentiation/vecjac_products.jl 89.28% <100.00%> (+8.47%) ⬆️
src/differentiation/common.jl 59.52% <59.52%> (ø)

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@avik-pal
Copy link
Contributor Author

@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
Copy link
Member

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants