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

mapPos provides wrong indices on submatrices #63

Open jk977 opened 4 years ago

jk977 commented 4 years ago

Overview

Using mapPos on submatrices does not generate the expected indices. I'm fairly sure this occurs because the function doesn't factor in the column offset when calling decode.

Versions

Example

The following gives unexpected results:

import qualified Data.Matrix as M
m = M.identity 3
sm = M.submatrix 2 3 1 2 m
unexpected = M.mapPos const sm

This assigns the following to unexpected:

┌             ┐
│ (2,2) (3,1) │
│ (4,1) (4,2) │
└             ┘

This could have one of two possible expected results. The first is to have the upper-left element be (1,1):

┌             ┐
│ (1,1) (1,2) │
│ (2,1) (2,2) │
└             ┘

The second is to have the upper-left element be (2,1), consistent with the submatrix offset:

┌             ┐
│ (2,1) (2,2) │
│ (3,1) (3,2) │
└             ┘

In my opinion, the first option provides more consistent results. It would prevent divergent behavior when, for example, calling mapPos on a 2x2 submatrix vs. a 2x2 matrix with no internal offsets.

jk977 commented 4 years ago

Quick update: if the first option was chosen, I'm thinking the matrix would need to be forced before mapping, unless the callback was changed from type (Int, Int) -> a -> b to (Int, Int) -> a -> a. The former would sacrifice performance, but the latter is a breaking change to the API and would sacrifice flexibility. Although forcing would cause the function to take a performance hit, it could be mitigated slightly by checking if ncols is equal to vcols before forcing. The matrix is not a submatrix if they're equal, so no forcing would be necessary.