WIP: Clean up linear algebra code

[jiahao]

This PR attempts to clean up the base linear algebra code.

General principles:

• Throw specific Exceptions in favor of generic error() messages
• Remove redundant syntax, e.g. extraneous returns and semicolons
• Turn simple methods into one-liners
• Consolidate tests of matrix properties using assertion macros to throw errors for singular matrices, lack of positive definiteness, rank deficiency, etc.
• Fix typos

[IainNZ]

You really don't like explicit returns, do you? :D

[StefanKarpinski]

My general policy for explicit vs. implicit returns has been to use return when the expression being returned is so short that not having the return looks strange. This tends to be the case with single-letter variable names.

[jiahao]

My general policy for explicit vs. implicit returns has been to use return when the expression being returned is so short that not having the return looks strange.

I'd rather be consistent here. "Looking strange" is a criterion that tends to not scale well. Also, having any returns could look strange to, say, Matlab users.

You really don't like explicit returns, do you? :D

Back when I was a young Julian hatchling, I had a certain colleague whom excoriated me for using them since "you don't need to!!" As a result, I've since past the point of no returns.

[ViralBShah]

I prefer to have explicit return statements. Anyways, this is good stuff. Perhaps just squash the commits before merging.

@jiahao I guess this will interfere with your PR for a new API for pivoted factorizations. Should be easy enough to merge.

[johnmyleswhite]

FWIW, I think all returns should be explicit.

[jiahao]

FWIW, I think all returns should be explicit.

tbh idrc. I just want to be consistent, and it was easier to delete returns.

Maybe it's time for an internal style guide?

[johnmyleswhite]

I've been writing up a style guide for DataArrays/DataFrames. The biggest impediment to a more general style guide is that many of the main contributors to Julia disagree on style.

[jiahao]

The biggest impediment to a more general style guide is that many of the main contributors to Julia disagree on style.

Some languages have BDFLs for this reason... but the democratic aspect of doing this in Julia is nice.

Here's a not unreasonable style guide:

• Explicit returns should be used in every function where there is more than one decision branch produced by anything other than a ternary operator that produces a return. For example:

function mywrapper(A)
    if issym(A)
        B, c = symmetric_function(A)
        return B, c #syntactically required
    elseif isherm(A)
        B, Q, c = hermitian_function(A)
        return B, Q, c #would look asymmetric without explicit return
    end
end

function mywrapper2(A)
    for i=1:10
        change!(A)
        if norm(A) > 10; return NaN; end #syntactically required
    end
    return A
end

• Implicit returns should be used in functions with 0 or 1 decision branches produced by anything other than a ternary operator that leads to a return:

function mywrapper3(A)
    x, y = funct(A)
    y > 0 ? throw(SingularError()) : x
end

since these tend to be shorter and leaving out the return contributes to the terseness.

• Avoid chaining more than 2 ternary operators to shoehorn functions into the second category, since more than 2 uses of ternary ?: signficantly decreases readability. (I would be fine with restricting this to 1 also.)

I prefer this to mandating explicit returns always, since it uses the semantic features of the language in a way that interpolates gracefully to the one-liner syntax where a return would simply look silly:

iszero(v)=return all([w==0 for w in v])

[StefanKarpinski]

FWIW, I think all returns should be explicit.

I have occasionally regretted not making explicit returns necessary – i.e. implicit returns are always nothing. One-liners would be the exception, of course, since the right-hand-side is the returned expression there.

[IainNZ]

I think I always explicitly return for defensive programming reasons (and because all other languages I use do it) - having the return value depend on whichever statement executes last seemed less safe/stable.

[johnmyleswhite]

Agreed: one-liners are the only good exception in my book.

For me the biggest trouble with implicit returning is that it interacts with things whose return value isn't obvious until you really know the language -- like for loops. I remember an e-mail (that I couldn't find in the archives) from Tim Holy where he noted that adding an explicit return nothing improved the performance of a chunk of code. Clearly that's a bug, but I vaguely remember it came up because the return type of something Tim had written wasn't at all obvious.

So I'd be hugely in favor of a future release of Julia deprecating implicit returns in multi-line function definitions.

[davidssmith]

+1 for deprecating implicit returns. It is jarring to have code like that reads somewhat grammatically and then just have a hanging noun at the end.

Even the ternary example above y > 0 ? throw(SingularError()) : x requires two readings for me, since one usually expects an assignment there.

Also, not having explicit returns makes it very difficult to find all function return values with a regex.

Edit: for multi-liners I mean. Omitting returns in one-liners is most excellent.

[JeffBezanson]

Deprecating implicit returns simply isn't going to happen. When manipulating code, you might need to generate a block like

begin
   do_something()
   x
end

i.e. yield the value of x after doing some action. In your macro or whatever, you don't know if this block is going to land in tail position, so you don't know whether to write return x. Furthermore, the abstract syntax of function f() ... end and f() = ... need not be different. Requiring return becomes very artificial.

[kmsquire]

Just so that anyone reading this doesn't get the idea that everyone hates
implicit returns,
I happen to like them, and I often find that they make my code less
cluttered and easier to understand.

(I also used to do a bit of scala programming, so I'm used to using them.)

On Monday, November 18, 2013, Jeff Bezanson wrote:

Deprecating implicit returns simply isn't going to happen. When
manipulating code, you might need to generate a block like

begin
do_something()
x
end

i.e. yield the value of x after doing some action. In your macro or
whatever, you don't know if this block is going to land in tail position,
so you don't know whether to write return x. Furthermore, the _abstract_syntax of function
f() ... end and f() = ... need not be different. Requiring return becomes
very artificial.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/4830#issuecomment-28726383
.

[StefanKarpinski]

Deprecating implicit returns simply isn't going to happen.

That said, perhaps as a stylistic matter, we could prefer explicit returns by convention.

[ivarne]

When we have implicit returns I think it is good to use it, to keep developers alert that the feature is there. The bugs where a syntax misunderstanding is the cause are often hard to find. If new users ignore the feature, they will design APIs where the functions does not end in return Nothing, even though it was intended to.

[jiahao]

The style convention about using explicit vs implicit returns is clearly contentious and there's no solution that's going to make everyone happy. I'm going to go ahead with my proposed convention here, which is probably going to minimize absolute total annoyance and maximize the number of people who will be annoyed to some degree.

[StefanKarpinski]

This is so comprehensive, @jiahao. Your systematic thoroughness always impresses me.

[andreasnoack]

Some rebase leftover here. I also wonder if something else went wrong in the rebase. A lot of level 2 code is added after herk which is level 3.

[jiahao]

Yes, I noticed that after rebasing. Fixing.

[andreasnoack]

@jiahao Great work. I only have the minor comments already that I raised in the code.

[jiahao]

Oops, I had meant to say that I was done and would leave the changes for further inspection, but additionally merged it into master immediately.

[ViralBShah]

I think this was ready to merge anyways.