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

Stop transpose(A) \ lu(B)' from overwriting A #36657

Merged
merged 5 commits into from
Jul 16, 2020

Conversation

dlfivefifty
Copy link
Contributor

@dlfivefifty dlfivefifty commented Jul 14, 2020

This fixes the following bug I just found:

julia> A = randn(5,5)
5×5 Array{Float64,2}:
  0.485776   1.29655    0.0790172   0.66021   -1.49472
  0.971676  -1.01382    0.630476    0.479027  -0.843428
 -0.264609   0.392383   0.646729    0.510696   0.34913
 -0.795944  -2.47709   -1.81052    -1.0947    -0.30381
 -0.938873   2.16395   -1.33484    -0.745461   1.43709

julia> transpose(A) / lu(randn(5,5))' # should not change A
5×5 Adjoint{Float64,Array{Float64,2}}:
  14.36      11.9797   -7.32563    1.87016   -16.2731
 -18.4415   -13.7036   10.6455    -2.47396    19.9999
   8.0401     8.31723  -5.16714    2.13261    -9.637
   4.19849    3.9865   -1.98478    0.714778   -4.62445
  -5.36059   -2.60991   0.917052   0.290281    4.62547

julia> A # but does!
5×5 Array{Float64,2}:
  14.36     -18.4415    8.0401    4.19849   -5.36059
  11.9797   -13.7036    8.31723   3.9865    -2.60991
  -7.32563   10.6455   -5.16714  -1.98478    0.917052
   1.87016   -2.47396   2.13261   0.714778   0.290281
 -16.2731    19.9999   -9.637    -4.62445    4.62547

This fixes the following bug I just found:
```julia
julia> A = randn(5,5)
5×5 Array{Float64,2}:
  0.485776   1.29655    0.0790172   0.66021   -1.49472
  0.971676  -1.01382    0.630476    0.479027  -0.843428
 -0.264609   0.392383   0.646729    0.510696   0.34913
 -0.795944  -2.47709   -1.81052    -1.0947    -0.30381
 -0.938873   2.16395   -1.33484    -0.745461   1.43709

julia> transpose(A) / lu(randn(5,5))' # should not change A
5×5 Adjoint{Float64,Array{Float64,2}}:
  14.36      11.9797   -7.32563    1.87016   -16.2731
 -18.4415   -13.7036   10.6455    -2.47396    19.9999
   8.0401     8.31723  -5.16714    2.13261    -9.637
   4.19849    3.9865   -1.98478    0.714778   -4.62445
  -5.36059   -2.60991   0.917052   0.290281    4.62547

julia> A # but does!
5×5 Array{Float64,2}:
  14.36     -18.4415    8.0401    4.19849   -5.36059
  11.9797   -13.7036    8.31723   3.9865    -2.60991
  -7.32563   10.6455   -5.16714  -1.98478    0.917052
   1.87016   -2.47396   2.13261   0.714778   0.290281
 -16.2731    19.9999   -9.637    -4.62445    4.62547
```
Still need to add tests.
@dlfivefifty dlfivefifty changed the title Stop transpose(A) \ lu(B)' from overwriting A Stop transpose(A) \ lu(B)' from overwriting A Jul 14, 2020
@dkarrasch dkarrasch added domain:linear algebra Linear algebra kind:bugfix This change fixes an existing bug labels Jul 14, 2020
@dkarrasch
Copy link
Member

dkarrasch commented Jul 15, 2020

I'd suggest to backport this. EDIT: Interestingly, the problem does not exist on v1.0.5.

@dkarrasch
Copy link
Member

Test failures seem unrelated...

@mbauman mbauman merged commit 94398d1 into JuliaLang:master Jul 16, 2020
JeffBezanson pushed a commit that referenced this pull request Jul 21, 2020
* Stop transpose(A) \ lu(B)' from overwriting `A`

This fixes the following bug I just found:
```julia
julia> A = randn(5,5)
5×5 Array{Float64,2}:
  0.485776   1.29655    0.0790172   0.66021   -1.49472
  0.971676  -1.01382    0.630476    0.479027  -0.843428
 -0.264609   0.392383   0.646729    0.510696   0.34913
 -0.795944  -2.47709   -1.81052    -1.0947    -0.30381
 -0.938873   2.16395   -1.33484    -0.745461   1.43709

julia> transpose(A) / lu(randn(5,5))' # should not change A
5×5 Adjoint{Float64,Array{Float64,2}}:
  14.36      11.9797   -7.32563    1.87016   -16.2731
 -18.4415   -13.7036   10.6455    -2.47396    19.9999
   8.0401     8.31723  -5.16714    2.13261    -9.637
   4.19849    3.9865   -1.98478    0.714778   -4.62445
  -5.36059   -2.60991   0.917052   0.290281    4.62547

julia> A # but does!
5×5 Array{Float64,2}:
  14.36     -18.4415    8.0401    4.19849   -5.36059
  11.9797   -13.7036    8.31723   3.9865    -2.60991
  -7.32563   10.6455   -5.16714  -1.98478    0.917052
   1.87016   -2.47396   2.13261   0.714778   0.290281
 -16.2731    19.9999   -9.637    -4.62445    4.62547
```

Co-authored-by: Daniel Karrasch <[email protected]>
(cherry picked from commit 94398d1)
simeonschaub pushed a commit to simeonschaub/julia that referenced this pull request Aug 11, 2020
* Stop transpose(A) \ lu(B)' from overwriting `A`

This fixes the following bug I just found:
```julia
julia> A = randn(5,5)
5×5 Array{Float64,2}:
  0.485776   1.29655    0.0790172   0.66021   -1.49472
  0.971676  -1.01382    0.630476    0.479027  -0.843428
 -0.264609   0.392383   0.646729    0.510696   0.34913
 -0.795944  -2.47709   -1.81052    -1.0947    -0.30381
 -0.938873   2.16395   -1.33484    -0.745461   1.43709

julia> transpose(A) / lu(randn(5,5))' # should not change A
5×5 Adjoint{Float64,Array{Float64,2}}:
  14.36      11.9797   -7.32563    1.87016   -16.2731
 -18.4415   -13.7036   10.6455    -2.47396    19.9999
   8.0401     8.31723  -5.16714    2.13261    -9.637
   4.19849    3.9865   -1.98478    0.714778   -4.62445
  -5.36059   -2.60991   0.917052   0.290281    4.62547

julia> A # but does!
5×5 Array{Float64,2}:
  14.36     -18.4415    8.0401    4.19849   -5.36059
  11.9797   -13.7036    8.31723   3.9865    -2.60991
  -7.32563   10.6455   -5.16714  -1.98478    0.917052
   1.87016   -2.47396   2.13261   0.714778   0.290281
 -16.2731    19.9999   -9.637    -4.62445    4.62547
```

Co-authored-by: Daniel Karrasch <[email protected]>
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:linear algebra Linear algebra kind:bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants