Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Fix Inverse for LUFactorized #297

Merged
merged 4 commits into from
Jun 12, 2021

Conversation

jturner314
Copy link
Member

This is a hack to fix a.factorize_into()?.inv()? when a is column-major. The reason why this corrects the result is complicated and is related to the fact that the a field in LUFactorized represents the LU factors of A^T in a very confusing way when the input A to the factorization was row-major. The solve modules need an overhaul to properly handle layouts in a clear way.

@jturner314 jturner314 added the bug Bug issue, or bug fix change label May 28, 2021
@codecov
Copy link

codecov bot commented May 28, 2021

Codecov Report

Merging #297 (6766c31) into master (082f01d) will increase coverage by 0.31%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
+ Coverage   89.01%   89.33%   +0.31%     
==========================================
  Files          71       71              
  Lines        3578     3610      +32     
==========================================
+ Hits         3185     3225      +40     
+ Misses        393      385       -8     
Impacted Files Coverage Δ
lax/src/solve.rs 100.00% <100.00%> (ø)
ndarray-linalg/src/solve.rs 78.57% <100.00%> (+5.99%) ⬆️
ndarray-linalg/tests/inv.rs 97.82% <100.00%> (+3.38%) ⬆️
ndarray-linalg/src/lobpcg/svd.rs 86.58% <0.00%> (+1.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 082f01d...6766c31. Read the comment docs.

This fixes the result of `a.factorize_into()?.inv()?` when `a` is
column-major.
The correct interpretation of the fields of LUFactorized is very
complicated when the input to the factorization was row-major.
@jturner314 jturner314 force-pushed the fix-factorize_into_inv branch from 8d7f43a to 6766c31 Compare May 28, 2021 09:22
@termoshtt termoshtt merged commit cd92891 into rust-ndarray:master Jun 12, 2021
termoshtt added a commit that referenced this pull request Jun 12, 2021
@jturner314 jturner314 deleted the fix-factorize_into_inv branch June 13, 2021 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug issue, or bug fix change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants