arrayfire / arrayfire-haskell

Haskell bindings to ArrayFire
http://hackage.haskell.org/package/arrayfire
BSD 3-Clause "New" or "Revised" License
59 stars 5 forks source link

Show instance transposed? #22

Closed gilgamec closed 4 years ago

gilgamec commented 4 years ago

It looks like your Show instance for Array is transposed:

λ> A.lower (A.matrix (2,2) [[1,2],[3,4]]) False
ArrayFire Array
[2 2 1 1]
    1.0000     2.0000 
    0.0000     4.0000 

λ> A.upper (A.matrix (2,2) [[1,2],[3,4]]) False
ArrayFire Array
[2 2 1 1]
    1.0000     0.0000 
    3.0000     4.0000 

Another example: exceptions thrown by matmul suggest that the dimensions are (rows,columns):

λ> A.matmul (A.constant [2,3] 5) (A.constant [1,2] 2) A.None A.None
*** Exception: AFException {afExceptionType = SizeError, afExceptionCode = 203, afExceptionMsg = "Invalid input size"}
λ> A.matmul (A.constant [2,3] 5) (A.constant [3,1] 2) A.None A.None
ArrayFire Array
[2 1 1 1]
   30.0000    30.0000 

but the Show instance suggests the opposite:

λ> (A.constant [2,3] 5)
ArrayFire Array
[2 3 1 1]
    5.0000     5.0000 
    5.0000     5.0000 
    5.0000     5.0000 
dmjio commented 4 years ago

Hmm, so there is an option to transpose right before printing, which is disabled by default.

instance Show (Array a) where                                                                                                                                          
  show = arrayString 

arrayString                                                                                                                                                            
  :: Array a                                                                                                                                                           
  -- ^ Input 'Array'                                                                                                                                                   
  -> String                                                                                                                                                            
  -- ^ 'String' representation of 'Array'                                                                                                                              
arrayString a = arrayToString "ArrayFire Array" a 4 False  -- <-- here

arrayToString expr (Array fptr) (fromIntegral -> prec) (fromIntegral . fromEnum -> transpose) =                                                                            
  unsafePerformIO . mask_ . withForeignPtr fptr $ \aptr ->                                                                                                             
    withCString expr $ \expCstr ->                                                                                                                                     
      alloca $ \ocstr -> do                                                                                                                                            
        throwAFError =<< af_array_to_string ocstr expCstr aptr prec transpose                                                                                              
        peekCString =<< peek ocstr  

--- determines whether or not to transpose the array before storing it in the string
gilgamec commented 4 years ago

Interestingly, it looks like the same is true in the arrayfire toString function; passing false transposes the array (relative to af_print) while passing true prints the same as af_print. Since the default value is true, I think the intended use is "false = transpose, true = don't transpose". (The documentation page only says the flag "determines whether or not to transpose the array before storing it in the string", not which is which. Boolean blindness strikes again!)

Anyway, it looks like this just needs to have the view pattern changed to fromIntegral . fromEnum . not. Or have the default in arrayString be True.

dmjio commented 4 years ago

Ah, it appears ArrayFire assumes data is in column major order. I think the Haskell wrapper might need to transpose the data first, before sending the C array to arrayfire. Since the smart constructors we expose seem to give the impression that data is in row major order.

matrix :: AFType a => (Int,Int) -> [[a]] -> Array a                                                                                                                    
matrix (x,y)                                                                                                                                                           
  = mkArray [x,y]                                                                                                                                                      
  . concat                                                                                                                                                             
  . take x                                                                                                                                                             
  . fmap (take y)                                                                                                                                                      
dmjio commented 4 years ago

@gilgamec also, one thing to note, transpose is the default for af_print, but the Haskell wrapper will first call af_array_to_string before printing, and specifying False /should not/ perform a transpose. Here's an example I have, one using af_print, and another using putStrLn w/ af_array_to_string.

main :: IO ()
main = do
    print newArray -- af_array_to_string
    printArray newArray -- af_print
  where
    newArray = matrix @Double (2,2) [ [1..], [1..] ] * matrix @Double (2,2) [ [2..], [2..] ]
ArrayFire Array
[2 2 1 1]
    2.0000     6.0000 
    2.0000     6.0000 

No Name Array
[2 2 1 1]
    2.0000     2.0000 
    6.0000     6.0000 

The second is transposed, the first is not.

gilgamec commented 4 years ago

So, going off upper and lower, it looks like we want to print transposed? As it seems to work as expected:

λ> A.printArray (A.upper arr False)
No Name Array
[2 2 1 1]
         1          3 
         0          4 

It looks like the printer outputs dimension 0 values to a single line, with each successive dimension adding a newline after. If the values are stored column-major (i.e. dimension 0 is the columns), you would indeed need to transpose so that dimension 0 is the rows and each row is printed to a single line.

dmjio commented 4 years ago

Alright, let's do it.

dmjio commented 4 years ago

25