Skip to content

WIP: Iterate Factorizations to allow deconstruction#25187

Closed
andreasnoack wants to merge 2 commits intomasterfrom
anj/nomatlab
Closed

WIP: Iterate Factorizations to allow deconstruction#25187
andreasnoack wants to merge 2 commits intomasterfrom
anj/nomatlab

Conversation

@andreasnoack
Copy link
Copy Markdown
Member

@andreasnoack andreasnoack commented Dec 19, 2017

As a followup to #25184 I thought I'd open a PR with (inferrable) Factorization deconstruction as suggested in JuliaLang/LinearAlgebra.jl#322 (and maybe other places) which could potentially clean up the linear algebra namespace. For now I've just implemented deconstruction of LU such that lufact can behave like lu, i.e.

julia> F = Base.LinAlg.lufact(randn(3,3))
LBase.LinAlg.LU{Float64,Array{Float64,2}}
L factor:
3×3 Array{Float64,2}:
  1.0        0.0       0.0
  0.453716   1.0       0.0
 -0.563746  -0.416037  1.0
U factor:
3×3 Array{Float64,2}:
 -2.01319   1.40547   1.30838
  0.0      -1.69498  -2.22183
  0.0       0.0      -0.409299

julia> L, U, p = F
Base.LinAlg.LU{Float64,Array{Float64,2}}
L factor:
3×3 Array{Float64,2}:
  1.0        0.0       0.0
  0.453716   1.0       0.0
 -0.563746  -0.416037  1.0
U factor:
3×3 Array{Float64,2}:
 -2.01319   1.40547   1.30838
  0.0      -1.69498  -2.22183
  0.0       0.0      -0.409299

julia> L
3×3 Array{Float64,2}:
  1.0        0.0       0.0
  0.453716   1.0       0.0
 -0.563746  -0.416037  1.0

julia> U
3×3 Array{Float64,2}:
 -2.01319   1.40547   1.30838
  0.0      -1.69498  -2.22183
  0.0       0.0      -0.409299

julia> p
3-element Array{Int64,1}:
 2
 1
 3

So if we think this is a good idea, we'd be able to deprecate at least lu, qr, eig, and svd.

@andreasnoack andreasnoack added linear algebra Linear algebra needs decision A decision on this change is needed labels Dec 19, 2017
@StefanKarpinski
Copy link
Copy Markdown
Member

StefanKarpinski commented Dec 19, 2017

I like it. Or we could just rename lufact to lu? But perhaps the longer names are better since then you can use lu as the name of a variable, which is kind of nice.

@andreasnoack
Copy link
Copy Markdown
Member Author

andreasnoack commented Dec 19, 2017

I'd be fine with either but I think the least disruptive change would be to first deprecate lu, qr, etc. and then maybe rename lufact to lu later.

@StefanKarpinski
Copy link
Copy Markdown
Member

StefanKarpinski commented Dec 19, 2017

That seems like more of a pain since with this change they become interchangeable and you'd want to deprecate the one that you're ultimately going to get rid of, no?

@Sacha0
Copy link
Copy Markdown
Member

Sacha0 commented Dec 25, 2017

Perhaps rebase with #25184 in?

@andreasnoack andreasnoack mentioned this pull request Dec 28, 2017
Closed
@Sacha0
Copy link
Copy Markdown
Member

Sacha0 commented Apr 4, 2018

Perhaps worth rebasing over the new iteration protocol? Best!

@nalimilan nalimilan added deprecation This change introduces or involves a deprecation triage This should be discussed on a triage call labels Apr 25, 2018
@Keno
Copy link
Copy Markdown
Member

Keno commented Apr 26, 2018

This seems useful, but is entirely within LinAlg. If somebody wants to get it in before the feature freeze, that'd be fine, but it's not blocking.

@Keno Keno removed the triage This should be discussed on a triage call label Apr 26, 2018
@Sacha0
Copy link
Copy Markdown
Member

Sacha0 commented May 5, 2018 •
edited
Loading

Some observations relevant to seeing this work through:

  1. qr accepts a full keyword determining the returned Q's form.
  2. qr(v::AbstractVector) methods for vector polar decomposition exist.
  3. lq accepts a full keyword like that for qr.
    Edits:
  4. lu(x::Number) returns a tuple of numbers, whereas destructuring lufact(x::Number) returns a tuple of arrays.

@Sacha0 Sacha0 mentioned this pull request May 5, 2018
Closed
@Sacha0 Sacha0 mentioned this pull request May 21, 2018
Closed
@ViralBShah
Copy link
Copy Markdown
Member

ViralBShah commented May 30, 2018

Perhaps we can close this one in light of recent work?

@Sacha0 Sacha0 closed this May 30, 2018
@Sacha0 Sacha0 deleted the anj/nomatlab branch May 30, 2018 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

deprecation This change introduces or involves a deprecation linear algebra Linear algebra needs decision A decision on this change is needed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@andreasnoack @StefanKarpinski @Sacha0 @Keno @ViralBShah @nalimilan