search

scijs / ndarray-blas-level2

BLAS Level 2 operations for ndarrays
MIT License
9 stars 2 forks source link

Expanded Unit Tests #6

Closed tab58 closed 8 years ago

tab58 commented 8 years ago

I think we should expand the unit testing to cover more test cases. Currently we only run a couple of tests per function, which may not be sufficient to test for accuracy for some functions.

This library is mostly matrix-vector multiplication, so let's compare the output of the specialized functions (symmetric, banded, etc.) to the output of the general matrix-vector (GEMV) implementation. We can test the hell out of GEMV to make sure it's accurate. I suggest we run a few thousand randomly created matrices and vectors through the both specialized functions and GEMV and compare results.

We'll have to make specific matrix creation functions for specialized matrices, such as:

This would help testing functions for accuracy and edge cases as well.

rreusser commented 8 years ago

Agreed. ndarray-band + ndarray-fill can be a useful way to fill a band of an ndarray and reduce the need to deal with indices and offsets manually, not that it's that difficult. And short of a seedable random number generator we can rely on, I've in the past filled a band with something like Math.cos(0.12950125 * i) (or other more or less irrational number) to generate randomish numbers and prevent unexpected rare failures. A seedable random number generator would be best though.

tab58 commented 8 years ago

Those look like great packages to leverage. That ought to work for what I'm proposing.

Also, I agree on the seedable PRNG. I saw a JS Mersenne Twister implementation here that I think will work great. Mersenne Twister is already used in a lot of places it seems, so I think this would be fine for the function unit tests.

rreusser commented 8 years ago

Nice, yeah, that looks great. Was struggling to search npmjs.com for a good prng, but that looks like a good fit.

rreusser commented 8 years ago

Only risk is possible caveat on externally-controlled dependencies. Worst-case scenario is a dependency-meltdown, though it's (hopefully) exceedingly unlikely and doesn't necessarily mean paranoia and reinventing the wheel is necessary.

tab58 commented 8 years ago

PR #8 has these features in it. LMK what you think.

rreusser commented 8 years ago

Awesome. Looking good to me. This is a step forward. Thoughts/ideas:

If #9 looks good we can merge and publish. Need to think about what it means to publish this since (holy crap) it has a dependent: https://www.npmjs.com/package/gencov

Is each new function a minor version bump? (as opposed to a patch or a major bump)

rreusser commented 8 years ago

As I realize once again that the versioning issue is the whole reason for hypermodularity. Man, I'd love for these all to be their own independent repo, but it basically doubles the maintenance cost. :disappointed:

tab58 commented 8 years ago

Personally I like the way NPM suggests how to do semver. Based on this, I think once PR#9 gets committed, then it's a minor bump since all these changes don't necessarily break functionality.

tab58 commented 8 years ago

Regarding the independent function repos, personally I'm not a fan of them. If any consumers want just the one or two functions they need, they can require just those files. Plus, you're right on about the maintainability of all those repos.

I like having one or two larger dependencies than having to keep a library up to date with 100+ smaller deps. Plus, it's easier to have your program come crashing to a halt because of something like left-pad.

rreusser commented 8 years ago

Yeah. I think there's a happy medium. I think a logical unit is the right answer. For some things, that might mean more repos. For BLAS, I think there's a case to be made for either, but I'm fine with the current approach. Maybe just a minor bump on function additions. So we may quickly end up at 1.23.0, but that's fine. Should stabilize.

Will merge PR ASAP. No computer at the moment.