Skip to content

Support StaticArrays Properly #436

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 5 commits into from
Dec 6, 2023
Merged

Conversation

avik-pal
Copy link
Member

@avik-pal avik-pal commented Nov 29, 2023

Checklist

  • Appropriate tests were added
  • Any code changes were done in a way that does not break public API
  • All documentation related to code changes were updated
  • The new code follows the
    contributor guidelines, in particular the ScioML Style Guide and
    COLPRAC.
  • Any new documentation only uses public API

Additional context

Add any other context about the problem here.

Copy link

codecov bot commented Nov 29, 2023

Codecov Report

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

Comparison is base (9787717) 66.17% compared to head (21559ed) 66.35%.

Files Patch % Lines
src/factorization.jl 85.71% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #436      +/-   ##
==========================================
+ Coverage   66.17%   66.35%   +0.18%     
==========================================
  Files          27       27              
  Lines        2019     2039      +20     
==========================================
+ Hits         1336     1353      +17     
- Misses        683      686       +3     

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

@avik-pal avik-pal marked this pull request as ready for review December 5, 2023 17:35
@avik-pal
Copy link
Member Author

avik-pal commented Dec 5, 2023

@ChrisRackauckas this is ready

@avik-pal
Copy link
Member Author

avik-pal commented Dec 5, 2023

To summarize what this does:

  1. changes the default for StaticArrays from GMRES to LU for square and SVD for non-square (QR(..) \ b is not defined so can't default to that)
  2. In a bunch of places <fact>! was used, for SArray that is changed to fact.
  3. For NormalCholesky the instance was being created for A, which should have been for Symmetric(A' * A) -- unfortunately A * A pre-1.10 doesn't pass JET opt tests, so testing only 1.10 and above for that.

@ChrisRackauckas ChrisRackauckas merged commit 98a377d into SciML:main Dec 6, 2023
@avik-pal avik-pal deleted the ap/defaults branch December 6, 2023 13:47
# 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