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

WIP: Iterate Factorizations to allow deconstruction #25187

Closed
wants to merge 2 commits into from

Conversation

andreasnoack
Copy link
Member

As a followup to #25184 I thought I'd open a PR with (inferrable) Factorization deconstruction as suggested in #15573 (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
Member

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
Member Author

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
Member

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
Member

Sacha0 commented Dec 25, 2017

Perhaps rebase with #25184 in?

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

Sacha0 commented May 5, 2018

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.

@ViralBShah
Copy link
Member

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
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
Development

Successfully merging this pull request may close these issues.

6 participants