indigits / scirust

Scientific Computing Library in Rust
Apache License 2.0
264 stars 29 forks source link

We shouldn't implicitly insert default values? #22

Open daniel-vainsencher opened 9 years ago

daniel-vainsencher commented 9 years ago

Matrix::from_iter* etc all fill out the rest of the matrix with zeros if they run out of values early. I have several problems with this:

I don't mind opening a PR if we agree on the change.

shailesh1729 commented 9 years ago

zero padding is a typical use case in signal processing. e.g. in order to do N point FFT, you first extend the vector to nearest power of 2 (padding with zero) and then process. May be we can have two versions. a from_iter which forces that at least M x N values must be there. Another from_iter_with_zero_padding which allows zeros to be filled in the remaining cells. The first one won't require Zero trait while the second one will require. The function name would clarify the intent. In both cases we should continue to allow a source iterator which may have more values than the matrix can consume.

daniel-vainsencher commented 9 years ago

Well, allowing larger iterators still masks some errors. I would prefer to make the default method (or version with the shortest name...) as safe as possible, but reasonable people may disagree.

shailesh1729 commented 9 years ago

Here is an alternative. Rather than spending too much gray cells on what is the right API for matrix, work more on developing the libraries on top of it (linear algebra, statistics, signal processing etc.) Come back later to the matrix API and do a cleanup based on actual use cases.

daniel-vainsencher commented 9 years ago

I am happily contributing code, but mostly as I need it myself. I spent all of today writing numerical code against scirust, and raised issues mostly based on that. This one specifically because I was missing a Zero trait in some context, hence the latest PR.