search

Daniel-Diaz / matrix

A Haskell native implementation of matrices and their operations.
BSD 3-Clause "New" or "Revised" License
35 stars 31 forks source link

Matricies are 1-indexed #23

Open recursion-ninja opened 8 years ago

recursion-ninja commented 8 years ago

I noticed that you had an indexing error in your library. If I want to get the element in the fist column of the first row I would need to erroneously call m ! (1,1) rather than the correct expression m ! (0,0). As you can see from the Vector library that you used internally, sequential data structures 0-indexed.

I'm sure that this is just a simple mistake in your internal encode function definition. While this does constitute a pretty serious defect in the library, it was probably just a simple off by one error that can be fixed quickly in version 4.0.0.0!

Daniel-Diaz commented 8 years ago

This was actually intentional. As a mathematician, I'm used to index matrices starting from 1. This is of course a design choice that some people like, and some people don't. It was no mistake. Although we can argue which one is preferable from different point of views.

The main issue here is that, as you can see from the issue list, I haven't been doing a good job maintaining this particular library. Mainly because of time concerns. I have many libraries (both public and private) under my supervision. I'll allocate some time this weekend to approach the different issues open and make a release. But it won't include a drastic change like the one proposed here.

And yes, changing encode and decodewould be the first step towards changing indexing if we decide to do that.

Thanks!

octopuscabbage commented 8 years ago

Keep matrices 1 indexed. Just put it more clearly in the documentation for us computer scientists.

istathar commented 8 years ago

Keep matrices 1 indexed. It's not like for (i=0;;i++) is idiomatic in our language. It's trivially easy to do a list comprehension [x | x <- [1..]] to get the indices one requires. Never hurts to increase clarity, but I must admit that I noticed the 1-indexing on my first read of your library's Haddock page, so it's not like it isn't there.

recursion-ninja commented 5 years ago

Because your library exposes many functions which require Vector as input values and produces Vector as output values, and Vector is zero indexed, your library exposes an inconsistent API to program against.

When working with Vector values to supply to a Matrix, or Vector value I receive from a Matrix function, I must use zero-indexed algorithms. However, when interacting with Matrix values, I must adapt all my algorithms to be one-indexed. This is a terrible user experience. Please consider adopting the standard indexing scheme of your package dependencies!

Because you do not do so, your package does not satisfy the following intuitive law, because the user must translate between two numbering schemes:

matrix ! (i,j) === (getRow i matrix) ! j
istathar commented 5 years ago

That's a fair argument.