fsharp icon indicating copy to clipboard operation
fsharp copied to clipboard

Type syntax for 5d, 6d etc. arrays

Open dsyme opened this issue 4 years ago • 6 comments

DiffSharp has need for 5D and 6D array types, e.g. types int[,,,,] and int[,,,,,].

These are actually allowed in F# through the syntax using explicit double back ticks:

int ``[,,,,]``

However the syntax int[,,,,] should be supported directly

Note that this is worth having even if we don't have Array4D and Array5D modules in FSharp.Core, as you can then write a module of operations like this:

module Array5D =
    let zeroCreate<'T> (length1:int) length2 length3 length4 length5 : 'T ``[,,,,]`` =
        let result = System.Array.CreateInstance(typeof<'T>, [|length1;length2;length3;length4;length5|])
        result :?> 'T ``[,,,,]``

    let get (array:'T ``[,,,,]``) (index1:int) index2 index3 index4 index5 : 'T =
        let result = array.GetValue([|index1;index2;index3;index4;index5|])
        result :?> 'T

    let set (array:'T ``[,,,,]``) (index1:int) index2 index3 index4 index5 (value: 'T) =
        array.SetValue(box value, [|index1;index2;index3;index4;index5|])
   
    let length1 (array:'T ``[,,,,]``) = array.GetLength(0)
    let length2 (array:'T ``[,,,,]``) = array.GetLength(1)
    let length3 (array:'T ``[,,,,]``) = array.GetLength(2)
    let length4 (array:'T ``[,,,,]``) = array.GetLength(3)
    let length5 (array:'T ``[,,,,]``) = array.GetLength(4)

    let init<'T> (length1:int) length2 length3 length4 length5 (initializer:int->int->int->int->int->'T) : 'T ``[,,,,]`` =
        let arr = zeroCreate<'T> length1 length2 length3 length4 length5
        for i1=0 to length1-1 do
            for i2=0 to length2-1 do
                for i3=0 to length3-1 do
                    for i4=0 to length4-1 do
                        for i5=0 to length5-1 do
                            set arr i1 i2 i3 i4 i5 (initializer i1 i2 i3 i4 i5)
        arr

dsyme avatar Aug 23 '21 12:08 dsyme

(We can conisder this a pre-apoproved language suggestion, only a tracking RFC needed)

dsyme avatar Aug 23 '21 12:08 dsyme

All that's needed to implement this is adding the appropriate lines here:

arrayTypeSuffix:
  | LBRACK RBRACK 
      { 1 }

  | LBRACK COMMA RBRACK 
      { 2 }

  | LBRACK COMMA COMMA RBRACK 
      { 3 }

  | LBRACK COMMA COMMA COMMA RBRACK 
      { 4 }

dsyme avatar Aug 23 '21 12:08 dsyme

Separately I noticed we hit an error path "rankOfArrayTyconRef: unsupported array rank" in hover tips in the IDE when defining these type abbreviations:

type 'T array5d = 'T ``[,,,,]``
type 'T array6d = 'T ``[,,,,,]``

The error path is the one in this code. THe problem is that some code in the IDE services is passing in the abbreviation tyconref for array5d into this, but the code is only expecting to be called with canonical (unabbreviated) array tycon ref.

let rankOfArrayTyconRef (g: TcGlobals) tcref =
    match g.il_arr_tcr_map |> Array.tryFindIndex (tyconRefEq g tcref) with
    | Some idx ->
        idx + 1
    | None ->
        failwith "rankOfArrayTyconRef: unsupported array rank"

For anyone using 5d/6d arrays - if you get the above error message you can simply remove the use of abbreviations.

dsyme avatar Aug 23 '21 13:08 dsyme

I tried to reproduce the error, but I was getting correct hover tips up to arrays of 32 dimensions. That was the max. that could be defined in F#. image

Practically, 12D is the last one which can be created using Array.CreateInstance => this would be the pragmatic limit.

Shall we also code-gen modules for each of the ranks? Or we leave it to be implemented as a library / copypaste file (would be my recommendation TBH) ?

T-Gro avatar Sep 22 '22 13:09 T-Gro

Not sure when https://github.com/dotnet/fsharp/pull/13700 will be in VS but in theory from now on for multidimensional arrays you will see array2d and so on up to 32 I believe .

See https://github.com/dotnet/fsharp/blob/342a6d2564d4718acfdfadce7130b3fd1510d732/tests/FSharp.Compiler.ComponentTests/Signatures/ArrayTests.fs#L35

edgarfgp avatar Sep 22 '22 13:09 edgarfgp

Pls correct me if I am wrong, but that looks like for being suited for "arrays of arrays of ..", which are irregular. Whereas the example module from don above is a regular array, with dimensions having strict lengths.

=> supporting [,,,,,,] up to 12D (limitation of System.Array.CreateInstance) still makes sense to me, even after #13700 . What are your thoughts @edgarfgp , @dsyme ?

T-Gro avatar Sep 22 '22 13:09 T-Gro

@T-Gro I think array12d is more readable that [,,,,,,,,,,,] in tooltips. and by using the same convention for types will be consistent :)

edgarfgp avatar Sep 23 '22 13:09 edgarfgp

I might be misreading the implementation of the recently merged feature, but to me it appears to be working with jagged arrays (arrays of arrays), which is a different thing then .NET multidimension arrays.

If I am understanding the newly introduced active pattern, is capturing array<array<array<.. and display that as an n-D array, correct?

T-Gro avatar Sep 23 '22 13:09 T-Gro

Here is an example using the latest feature (launched with current main of the compiler). Problem is, we now have two different types (jagged arrays, and classical multidimension arrays), both being rendered as array3d (ditto for higher dimensions).

I do like the new feature developed by @edgarfgp , I just think we should use distinct naming in higher dimensions. Originally, F#'s Array2,3,4D arrays and their modules were about classical multidimensional arrays. The new feature has better tooling for jagged arrays (e.g. for array<array<array<array<array<array<array<array>>>>>>> ). My concern is that we should use a different name, not make it sure that jagged/nested arrays and multi-dimensional arrays are not confused.

let newFeature3dJagged : array<array<array<int>>> = Unchecked.defaultof<_>
newFeature3dJagged[1,2,3] <- 15 // error saying array3d and array3d do not match => this is bad, we should not have two different standard types with the same name
newFeature3dJagged.[1].[2].[3] <- 15 // this works just fine, because this is a jagged array

let classicalMultidimension3D  = Array3D.zeroCreate 1 1 1
classicalMultidimension3D[1,2,3] <- 15 // works ok

The error message here is the bad part, since both types are called with the same name, despite being different types (in .NET runtime) and offering different indexing mechanism. Jagged/nested arrays allow indexing 1 dimension at a time, whereas true multidimensional arrays offer indexing by all dimension within a single pair of brackets. image

@dsyme I would like to ask for your view on the naming. I think that the recent (already merged) PR from @edgarfgp is good, we should just decide on naming for jagged arrays and not call them arrayNd, because that is:

  • already used by array2d, array3d, array4d
  • we should keep it for this proposal of yours for higher dimensions.

T-Gro avatar Sep 23 '22 14:09 T-Gro

I shared the credit/blame 😅 of #13700 with @nojaf .

edgarfgp avatar Sep 23 '22 14:09 edgarfgp

Jagged arrays should not have changed printing with 13700, it would be a mistake if they have

dsyme avatar Sep 23 '22 16:09 dsyme

Good. Now that the naming conflict is resolved, I will assign this issue to myself.

@dsyme : Considering the DiffSharp support, did you had particular module functions and slicing support in mind as well? Or mainly about having the option to define the type and index into its values?

T-Gro avatar Sep 26 '22 07:09 T-Gro