doc: add factorizations docstrings #31284

dkarrasch · 2019-03-07T10:27:48Z

Contributes to #31202.

I have added a very short docstring to Factorization, and updated the manual. Now, all subtypes(Factorization) are listed, except the three SuiteSparse ones. I couldn't find LUTridiagonal anywhere, so I deleted it from the manual. This certainly needs a careful review to make everything consistent. For example, would the reference to the online manual work like that? Once all types have their own docstrings, should the table in the manual contain references? ~~Should we keep this PR restricted to Factorization, or should we collect docstrings for each concrete factorization type here?~~ I went ahead with adding docstrings for the concrete factorization types.

dkarrasch · 2019-03-07T11:19:39Z

I'll leave it at the three factorizations, and continue once we agree on the style of the docstrings.

StefanKarpinski · 2019-03-07T13:39:56Z

Thank you so much for adding to the docs! This is so useful and deeply appreciated. ❤️

kshyatt · 2019-03-07T15:28:15Z

stdlib/LinearAlgebra/src/bunchkaufman.jl

+"""
+    BunchKaufman <: Factorization
+
+Matrix factorization type of the Bunch-Kaufman factorization of a `Symmetric` or


Can we x-ref Symmetric and/or Hermitian here?

Actually, I realized that this may be misleading. What you need is that the matrix is numerically symmetric or Hermitian, it doesn't have to be of Symmetric or Hermitian type. I'd suggest to type these expressions as regular text rather than as code. That's how it's done in cholesky as well.

kshyatt · 2019-03-07T15:29:13Z

Might be nice to include showing how access specific factors of the factorization in the examples?

dkarrasch · 2019-03-07T15:49:33Z

Might be nice to include showing how access specific factors of the factorization in the examples?

I excluded that on purpose, because that's demonstrated in the actual factorization function docstrings. My intention was to direct users to those functions, because that's how people would use it in practice.
EDIT: I copied the (first half of the) respective examples from those factorization functions, so if we decide to include that, we could simply use the entire example code.

I (now) agree. Accessing the factors is related to the return type, not necessarily to the factorization function. So, I'll add information on the factors in the docstring, and copy also the related code from the examples in the factorization functions.

dkarrasch · 2019-03-07T15:52:25Z

I also noticed that I appended <: Factorization to BunchKaufman, but forgot to do so in the other ones. I'll try to look up what is done in other similar cases, or gratefully accept advice.
EDIT: Indeed, I found similar subtype information, e.g., for Matrix <: AbstractMatrix in the Julia docs, so I guess it wouldn't hurt including that.

dkarrasch · 2019-03-08T09:53:57Z

Alright, I couldn't stop myself finishing all factorizations. I have consistently appended the supertype information, and added (i.e. copied) how to access the individual factors. Since the types are going to appear in the manual right above the corresponding factorization functions, there will be quite some redundancy there, but otherwise I think this is helpful information. Otherwise, one would have to explicitly refer the user to the docstrings of the factorization functions for access info. With this, I'm sort of done on my end, happily awaiting review and feedback.

andreasnoack

This is great work. Just a minor comment to the Bunch-Kaufman docstring.

andreasnoack · 2019-03-14T07:40:06Z

stdlib/LinearAlgebra/src/bunchkaufman.jl

+    BunchKaufman <: Factorization
+
+Matrix factorization type of the Bunch-Kaufman factorization of a symmetric or
+Hermitian matrix `A` as ``P'*U*D*U'*P`` or ``P'*L*D*L'*P``, depending on


It's a bit more general since it can also factorize complex symmetric matrices in which case it should be transpose and not '.

Since this is in math mode, can I use P^\\top or P^T? Or should I turn it simply into code mode and use transpose? That mixture of code and math looks a bit weird anyway. Normally, we wouldn't write the * in math mode, would we?

I missed to adopt the corresponding parts from the bunchkaufman docstring, because I initially felt that this relates more to the factorization function, and less to the factorization type (see above). But I agree that this should be mentioned, and I have added it. I also turned the math type into code type, and did the same to the factorization function docstring. There, I also changed the wording slightly, to reflect the fact that the matrix should be symmetric/Hermitian, not necessarily of Symmetric or Hermitian type. I hope that with the added doctest example, the issue with the stored triangle and the wrapping by the special types becomes clearer.

dkarrasch · 2019-04-08T08:25:57Z

CI says it can't find some of the references, but I have no idea why. The referenced types/commands have not been touched in this PR, and they do have docstrings. ~~Seems like I have bad karma with CI. :-(~~ No bad karma, just real mistakes.

dkarrasch · 2019-04-08T16:04:43Z

This is ready for review. All docstrings are written and included in the package documentation page. CI failure seems to be unrelated, AFAICT.

dkarrasch · 2019-05-10T18:11:19Z

Bump. I'd love to get rid of my open doc PRs. 😄

StefanKarpinski · 2019-05-12T17:22:34Z

Seems to have an unusually large number of CI failures...

dkarrasch · 2019-05-12T18:08:45Z

I know, but I couldn't relate the failures to my docstrings. :-(

StefanKarpinski · 2019-05-14T13:33:57Z

@staticfloat, should I be worried about any of these package_win{32,64} failures or the tester_macos64 failure? The former seem unrelated and the latter is cholmod-related, but I don't see how docstring changes could affect that, could they?

staticfloat · 2019-05-14T14:43:58Z

The failures show me that this branch hasn't been rebased on top of master in quite a while; so it hasn't picked up the build system improvements that we've made in the last few months; I suggest rebasing to bring along those build system improvements and fixes.

dkarrasch · 2019-05-14T14:50:41Z

Oho, do I simply need to do (when in this branch)

git fetch upstream
git rebase upstream/master

or is this going to affect my changes? Hopefully, I will only need to resolve potential merge conflicts...?

staticfloat · 2019-05-14T16:24:37Z

What I like to do is git pull --rebase origin/master. That should do it all in one go. If there are merge failures, you edit the affected files, fix the conflicts, then git add the fixed files and continue with git rebase --continue. Then, once you're finished and satisfied with things, you git push --force to force-update this branch to the newly-rebased contents.

dkarrasch · 2019-05-14T18:24:40Z

Sorry, I messed up a bit. I tried @staticfloat 's suggestion first, but git was complaining. Then I went on with my own guess, but I guess I should have force-pushed, and not just pushed (I pushed the button in Atom). Now git thinks that all the commits to master from the meantime are new. At least, my 21 commits are the last ones in the list. How can I tell git to consider only the last 21 as part of this PR? My apologies for the git ignorance...

codecov-io · 2019-05-14T19:03:53Z

Codecov Report

Merging #31284 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #31284      +/-   ##
==========================================
- Coverage   81.35%   81.34%   -0.01%     
==========================================
  Files         372      372              
  Lines       63035    63035              
==========================================
- Hits        51283    51278       -5     
- Misses      11752    11757       +5

Impacted Files	Coverage Δ
base/asyncmap.jl	`92.24% <0%> (-2.59%)`	⬇️
base/bitset.jl	`92.74% <0%> (-1.56%)`	⬇️
stdlib/SparseArrays/src/linalg.jl	`94.65% <0%> (ø)`	⬆️
stdlib/Random/src/generation.jl	`84.96% <0%> (+0.65%)`	⬆️

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 40d72ad...b9c936b. Read the comment docs.

staticfloat · 2019-05-14T22:00:34Z

Funnily enough, the solution to this is exactly the same as what I said before; just git pull --rebase origin master. (NOTE: I typed origin/master before, but you need to do origin master. Whoops.)

I tried to do it myself, but unfortunately there are enough git conflicts that it's not entirely obvious to me which should be chosen. If you have trouble with git, feel free to hit me up on the JuliaLang slack and I'll help you fight through them. Once you get used to rebasing in git, you won't be able to stop. :)

dkarrasch · 2019-05-15T08:30:25Z

I'm close to tears, I think it worked (and days (if not weeks! ;-)) of work are preserved).

One strange thing along the way was that when I git add stdlibs/.../file, and then git rebase --continue, git kept telling me each time that "nothing changed, maybe you forgot to git add?", and suggested to git rebase --skip. The latter sounds to me like "oh, just skip all of your changes, they may be not so important" 😃 , but I was brave enough to still "--skip". Looks like it included all the changes anyway. This looks like magic to me. Okay, now, as @staticfloat said, I'll go and rebase all my other old PRs. 🤣

StefanKarpinski · 2019-05-15T12:05:40Z

Failures are clearly unrelated now. It's just the usual inability of @appveyor and @travis-ci to do simple things like stat a local file in under 80 seconds or connect to the internet. Thank goodness we have our own CI now that actually works.

dkarrasch · 2019-05-23T17:37:41Z

I'm afraid I need some help with resolving the merge conflict, @stevengj. One of my contribution here was to add documentation for the Hessenberg type. It seems that this was also/already taken care of in #31853. Should I simply exclude all my changes in hessenberg.jl, or is there anything worth keeping? I'm fine with excluding the hessenberg changes, but then I'd appreciate a hint on how to do that in git, without messing up my big PR.

I ended up simply replacing my hessenberg-file by @stevengj's #31853, the merge conflicts were rather easy to resolve.

c42f · 2019-05-28T11:49:10Z

I was browsing (for something unrelated) but just noticed this amazing work. Fantastic!

A note on dealing with large rebases: if things were developed iteratively with lots of small fixes to the same parts of the code, I often find it's helpful just to give up on the unruly history of the branch and squash it all into one commit from which the conflicts can be dealt with a single time. It looks like you've solved the conflicts though which is great. Also, it's harder than you might think to truly loose things with git (provided you've committed them at least once). When I find myself in trouble with a rebase, git rebase --abort and git reflog have saved my sanity many a time ;-) If you suspect you've botched a rebase, you can always use the reflog to resurrect the old commit.

dkarrasch · 2019-05-28T12:09:44Z

This is good to go now. The only Travis failure seems to be Pkg and internet connection.

docs for BunchKaufman docs for Cholesky(Pivoted) docs for (Generalized)Eigen minor edits merge hessenberg docs for LDLt docs for LQ docs for LU merge new hessenberg add components to LQ docstring add components to LU docstring add components to Eigen docstring add components to GeneralizedEigen add components to Cholesky(Pivoted) add components to BunchKaufman docs for (Generalized)Schur docs for (Generalized)SVD fix bunchkaufman as per review list factorization types in index.md fix docstring x-refs

dkarrasch changed the title ~~doc: add Factorization docstring~~ doc: add factorizations docstrings Mar 7, 2019

kshyatt added docs This change adds or pertains to documentation linear algebra Linear algebra labels Mar 7, 2019

kshyatt requested a review from andreasnoack March 7, 2019 13:03

mbauman mentioned this pull request Mar 7, 2019

Documenting all exports from the standard libraries #31202

Closed

kshyatt reviewed Mar 7, 2019

View reviewed changes

andreasnoack reviewed Mar 14, 2019

View reviewed changes

dkarrasch mentioned this pull request Mar 29, 2019

Doc addition for SVD and Hessenberg #31529

Closed

dkarrasch closed this May 10, 2019

dkarrasch reopened this May 10, 2019

dkarrasch closed this May 14, 2019

dkarrasch reopened this May 14, 2019

StefanKarpinski merged commit 4160b00 into JuliaLang:master May 29, 2019

dkarrasch deleted the dk/factorization_docs branch May 30, 2019 10:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

doc: add factorizations docstrings #31284

doc: add factorizations docstrings #31284

dkarrasch commented Mar 7, 2019 •

edited

Loading

dkarrasch commented Mar 7, 2019

StefanKarpinski commented Mar 7, 2019

kshyatt Mar 7, 2019

dkarrasch Mar 7, 2019

dkarrasch Mar 7, 2019

kshyatt commented Mar 7, 2019

dkarrasch commented Mar 7, 2019 •

edited

Loading

dkarrasch commented Mar 7, 2019 •

edited

Loading

dkarrasch commented Mar 8, 2019

andreasnoack left a comment

andreasnoack Mar 14, 2019

dkarrasch Mar 14, 2019 •

edited

Loading

dkarrasch Mar 15, 2019

dkarrasch commented Apr 8, 2019 •

edited

Loading

dkarrasch commented Apr 8, 2019

dkarrasch commented May 10, 2019

StefanKarpinski commented May 12, 2019

dkarrasch commented May 12, 2019

StefanKarpinski commented May 14, 2019

staticfloat commented May 14, 2019

dkarrasch commented May 14, 2019

staticfloat commented May 14, 2019 •

edited

Loading

dkarrasch commented May 14, 2019

codecov-io commented May 14, 2019 •

edited

Loading

staticfloat commented May 14, 2019

dkarrasch commented May 15, 2019

StefanKarpinski commented May 15, 2019

dkarrasch commented May 23, 2019 •

edited

Loading

c42f commented May 28, 2019

dkarrasch commented May 28, 2019

doc: add factorizations docstrings #31284

doc: add factorizations docstrings #31284

Conversation

dkarrasch commented Mar 7, 2019 • edited Loading

dkarrasch commented Mar 7, 2019

StefanKarpinski commented Mar 7, 2019

kshyatt Mar 7, 2019

Choose a reason for hiding this comment

dkarrasch Mar 7, 2019

Choose a reason for hiding this comment

dkarrasch Mar 7, 2019

Choose a reason for hiding this comment

kshyatt commented Mar 7, 2019

dkarrasch commented Mar 7, 2019 • edited Loading

dkarrasch commented Mar 7, 2019 • edited Loading

dkarrasch commented Mar 8, 2019

andreasnoack left a comment

Choose a reason for hiding this comment

andreasnoack Mar 14, 2019

Choose a reason for hiding this comment

dkarrasch Mar 14, 2019 • edited Loading

Choose a reason for hiding this comment

dkarrasch Mar 15, 2019

Choose a reason for hiding this comment

dkarrasch commented Apr 8, 2019 • edited Loading

dkarrasch commented Apr 8, 2019

dkarrasch commented May 10, 2019

StefanKarpinski commented May 12, 2019

dkarrasch commented May 12, 2019

StefanKarpinski commented May 14, 2019

staticfloat commented May 14, 2019

dkarrasch commented May 14, 2019

staticfloat commented May 14, 2019 • edited Loading

dkarrasch commented May 14, 2019

codecov-io commented May 14, 2019 • edited Loading

Codecov Report

staticfloat commented May 14, 2019

dkarrasch commented May 15, 2019

StefanKarpinski commented May 15, 2019

dkarrasch commented May 23, 2019 • edited Loading

c42f commented May 28, 2019

dkarrasch commented May 28, 2019

dkarrasch commented Mar 7, 2019 •

edited

Loading

dkarrasch commented Mar 7, 2019 •

edited

Loading

dkarrasch commented Mar 7, 2019 •

edited

Loading

dkarrasch Mar 14, 2019 •

edited

Loading

dkarrasch commented Apr 8, 2019 •

edited

Loading

staticfloat commented May 14, 2019 •

edited

Loading

codecov-io commented May 14, 2019 •

edited

Loading

dkarrasch commented May 23, 2019 •

edited

Loading