Left vector multiply

[JeffFessler]

Closes #99 by providing * and mul! methods for adjoint/transpose vectors on left of a LinearMap, like y'*A.

[codecov]

Codecov Report

Merging #100 into master will increase coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   98.14%   98.29%   +0.15%     
==========================================
  Files          11       12       +1     
  Lines         809      822      +13     
==========================================
+ Hits          794      808      +14     
+ Misses         15       14       -1     

Impacted Files	Coverage Δ
src/LinearMaps.jl	98.50% <100.00%> (+1.44%)	⬆️
src/left.jl	100.00% <100.00%> (ø)

---

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 b06ba20...7fff78d. Read the comment docs.

[JeffFessler]

I found a bug in the tests. Still working on it...

[JeffFessler]

Ok, as I expected handling * was easy, but mul! is hard because it is difficult for me to follow the many variations of mul! throughout the package. The main change is that most of the existing mul! methods had AbstractVector as the target, and this has to be generalized to a Union that includes Adjoint or Transpose abstract vectors. (I called it VecOut.) I ended up isolating most of the new code to left.jl in src/ and in test/.
Now I need some help.
Firstly, I can't get it to work for 1.0 and honestly I'd rather not spend time trying to make new features work for an old Julia version. So my kludge is simply not to test the new features for old versions.
More importantly, the mul! works for several subtypes of LinearMaps, but not all subtypes, as seen in the notes in test/left.jl. Some types end up in infinite loops leading to stack overflow. (Multiple dispatch is powerful but also hard to trace what is happening...) Probably some of the issues for those types is that they use @eval to create some of their methods and I was less sure how to modify those.

[dkarrasch]

Thanks for working on this, @JeffFessler! I don't have time right now for a detailed review, but a quick suggestion.

[JeffFessler]

After improving check_dim_mul as @dkarrasch suggested, I was able to track down the remaining bugs and greatly simplify things. It turns out that the existing AbstractVector outputs were just fine for nearly all existing functions. So now everything works for all LinearMap types and code review will be easier (only 6 files changed instead of 14). And Julia 1.0 is 95% working now. It works fine for mul!(x, y', A) but not for mul!(x, transpose(y), A), oddly, so I have fenced out that test. I kind of doubt that anyone will ever use mul!(x, transpose(y), A) in any Julia version, but at least I have isolated the issue to a small part in case you want to take a stab at it. Obviously when the time comes I'd recommend squashing the commits because there were a lot of useless tries along the way...

[dkarrasch]

The issue with v1.0 was minor. You forgot to define a 3-arg mul! for the transpose(y)*A case. I know this is poorly documented (and improving that is on my todo list), but I decided in one of the more recent PRs to clearly distinguish between 3-arg and 5-arg calls. By default, we should not direct 3-arg calls to 5-arg mul!s as in the matrix case, because that is, in general, a bad idea for FunctionMaps. I also know that this seems to blow up the amount of mul! methods, but I think the bigger issue about that is lack of documentation. The "repetitions" follow a clear pattern, and if that's well-communicated, it looks much less scary. Having said that, I wouldn't mind if we could reduce it again, but I was having big troubles with this "5-arg default" in #74.

From my point of view, this is ready for a final quick review, and then I'll merge and release.

[JeffFessler]

Thumbs up on the duck typing for check_dim_mul and the more informative error messages.
I find such messages to be super helpful for debugging.
The one-line version using || seems to help with code coverage whereas the if/then approach leads to a missed coverage line unless you explicitly add another test to make it throw...
(As I'm sure you know; I went ahead and made the change.)

[dkarrasch]

Thumbs up on the duck typing for check_dim_mul and the more informative error messages.

Yeah, I basically copied them from LinearAlgebra.generic_matmul or something like that.

[dkarrasch]

Technically, this is a breaking change, right? So by the semver rules, this needs to get a new major version number. I'd suggest to put this together with #97 and perhaps #87 and make a v3.0 release?

I'm on vacation (offline) the next two weeks. Is there any hurry in getting this released?

[JeffFessler]

Waiting is fine. If needed I can use this branch in the meantime.

[JeffFessler]

Lots of nice updates.
How close are we to a new tagged version with this left vector multiply merge in it?