Added SBMV function and unit tests.

[tab58]

Referenced LAPACK library for logic: http://www.netlib.org/lapack/explore-html/d7/d15/group__double__blas__level2_ga5c7ca036c788c5fd42e04ade0dc92d44.html#ga5c7ca036c788c5fd42e04ade0dc92d44

[rreusser]

Added a couple notes. Overall LGTM. I support robustness first, followed by optimization.

One additional question: do you think we should throw errors if the dimensionality or dimension sizes are incompatible? I don't think we have so far, but I think it's the right move. The general conclusion before was to return false if the algorithm fails and throw an error if the operation is invalid. The actual amount of work involved is almost certainly negligible unless you're multiplying 2 x 2 matrices 10000000 times—which might not be that unrealistic but is probably a bad idea if it's actually the bottleneck.

[tab58]

K, before we hit the big green button, I realized my test for this is terrible. I was having some rebase issues in Git, so I had to do some manual file moving and force pushes to my fork. The tests aren't right for this. Let me fix this before we merge.

[tab58]

This should be fixed now. If this looks good to you @rreusser, hit the big green button. 😄

[rreusser]

Could you add a note on the storage format first? Coming to this library, I'd be unable to use the sbmv without either working through the code or locating another reference on the topic. We maybe don't need a whole code snippet, but we at least need to describe that the diagonal is in… row k of A? With superdiagonals above that? I'm not sure if that's correct, but we should at least describe it to the point that it's usable.

[rreusser]

Made a few comments. Hate to nitpick too much, but I think it's important to get the API and usage consistent and tight so that people don't have to worry about all the caveats and subtleties. Here's the things I'd like to tighten up in general, and I'm glad to help with the cleanup:

• Avoid redundant arguments (stated dimensions as well as the actual dimensions of input)
• Throw errors on invalid input (e.g. dimensions don't match; I think it's okay to skip checking if it's an ndarray)
• Leave out upper/lower arguments if it can be accomplished symbolically
• Return true in general (there's no possibility of failure here, but makes it more meaningful to return false for algorithms that can fail)
• Documentation includes a full description of storage and input/output. Target: can use without referring to either the source or another reference.

If that makes sense, let's merge this since it's a big step forward, then continue with building it out before publishing to npm.

And despite all that, please don't think I'm not grateful and excited that you're back to work on this!

[tab58]

You're just fine with the nitpicks. It's good that you're asking questions cause that means you're analyzing my code. 😄 I would hate to commit something unusable. Let me address your comments:

• Yeah, I don't want redundant arguments. In the sbmv case, maybe fromLower is somewhat redundant, given that A is stored as a regular dense matrix. I think the real impetus behind this (for me anyway) was a case where the different triangular portions of the matrix mean different things. For example, there is a way to perform LU decomposition and calculate the L and U matrices in place, so maybe you want to operate on it with a method like this. I don't know why this case would ever come up, but I could imagine some other slick algorithm doing something similar. Basically it was to make sure we didn't deny any options to the user. Besides, fromLower defaults to true if not supplied, so I thought I was safe. Maybe you have different thoughts on the matter.
• About documentation, you're right. I need to explain more about storage and what the function arguments mean. I'll go and do that. I always hate the documentation side, but who doesn't? :smile:
• I would definitely like to fail fast by argument mismatch. That will then force the user to change the ndarray stride/offset/whatever before using the functions. I'll change that too.

[tab58]

What are your feelings on using console.warn() for reasons the function failed, @rreusser?

[rreusser]

Hmm… based on past mistakes, let's avoid console.warn if we can. If you trap an error and choose to ignore it, you might still get tons of unwanted console output. Most people don't trap unexpected errors, so as long as you throw new Error('Useful message here'), it should show up in the console anyway.

[tab58]

So we do want to throw errors instead of console.warn()? That's cool. I'll do that.

[tab58]

But if we're throwing errors on argument mismatches, then why would we then want to return false?

[rreusser]

I like console.warn for things like a recursion limit reached where you might still get a meaningful answer but with caveats. And if you do that, I think the right way is to keep a module-global variable that counts how many times the error has been logged, and then omit further errors after some reasonable limit. Bottom line: except for that very specific case of true 'warnings' as opposed to errors or algorithm failures, it's a small bit of unpleasantness that I think is best avoided.

And as for errors, I think only throw errors on invalid input. So:

• incompatible dimensions = throw error
• invalid input type = throw error (though sometimes you can leave those out and it's hopefully obvious enough that .get wasn't a function on the number 7 because 7 is not an ndarray)

As opposed to:

• divide by zero or some other algorithm failure = return false. This is distinct from invalid input since the input may have been perfectly valid and it's the algorithm that's not capable of proceeding.

[rreusser]

The downside being that it's difficult to handle multiple return values (e.g. a numerical result + true/false representing success/failure—which you can do but it's kinda overloading the meaning of the return value such that any additional details are getting to be a bit much for one return value). One approach I've taken with that is to accept an optional status object as a final object. If provided, then additional information about the result is computed and passed back. See complex-deriv-fornberg for an example. (This could also be accomplished with a callback. I'm not sure how I feel about that.)

[rreusser]

To answer your specific question though, this algorithm should never need to return false. A return true at the end should be sufficient. Sorry, I was speaking in more general terms.

[tab58]

Added some things @rreusser. LMK if you have any issues with them.