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

Rename rank one up- and downdate functions for Cholesky to rank(up/down)date #14424

Merged
merged 1 commit into from
Dec 18, 2015

Conversation

andreasnoack
Copy link
Member

No description provided.

@jiahao
Copy link
Member

jiahao commented Dec 16, 2015

I dislike having the rank prefix because it is a property of the quantity you do the up/down-date with and is not essential to the idea of a up/down-date. In principle there is nothing stopping you from computing a up/down-date with a low rank matrix.

@timholy
Copy link
Member

timholy commented Dec 16, 2015

updateAAt!?

@andreasnoack
Copy link
Member Author

@jiahao I think it's pretty standard terminology for this operation, e.g. http://www.netlib.org/lapack/explore-html/d7/d15/group__double__blas__level2.html#ga35ca25bb135cd7bfdd5d6190b1aa4d07.

@timholy I don't like the name. I consider myself in a process of removing the As (and Bs) from Julia.

@jiahao
Copy link
Member

jiahao commented Dec 16, 2015

@andreasnoack I don't see how the LAPACK page you linked to substantiates this operation being called a "rank update". If anything, the documentation clearly describes a "rank 1" operation (the 1 is missing from the function name) and the notion of update is not described in the main documentation (only in the description of the arguments).

Again, my point is that the phrase "rank update" is meaningless. "rank 1" is the adjectival phrase attached to "update"; "rank" itself cannot meaningfully describe any kind of update. That's why functions like "syrk" have the k - they are to describe operations dealing with "symmetric rank k" (the k cannot be omitted and still be a meaningful English phrase).

How about cholupdate! and choldowndate!? I don't like them as much as down/update!(::Cholesky), but I think we have to give the genericity issue a concession (at least, until Base.update! exists).

@jiahao
Copy link
Member

jiahao commented Dec 16, 2015

I would even consider ranknupdate! less ugly than rankupdate!.

@andreasnoack
Copy link
Member Author

The k versus 1 is in the signature here.

@jiahao
Copy link
Member

jiahao commented Dec 16, 2015

But that's exactly my point regarding the name rankupdate, vs ranknupdate or rank1update.

@andreasnoack
Copy link
Member Author

After offline discussions, we have settled (locally) for lowrank(up/down)date. It's a bit long, but I prefer a slightly long name to a cryptic name with abbreviations.

andreasnoack added a commit that referenced this pull request Dec 18, 2015
Rename rank one up- and downdate functions for Cholesky to rank(up/down)date
@andreasnoack andreasnoack merged commit 754c792 into master Dec 18, 2015
@andreasnoack andreasnoack deleted the anj/rankupdate branch December 18, 2015 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants