Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Should Chapel support initializations of multidimensional arrays using arrays-of-arrays? #26813

Open
bradcray opened this issue Mar 1, 2025 · 9 comments

Comments

@bradcray
Copy link
Member

bradcray commented Mar 1, 2025

Today, a(n explicitly typed) multidimensional Chapel array can be initialized using a tuple of tuples. For example,

var A: [1..2, 1..2] real = ((1.1, 1.2), (2.1, 2.2));

print(A);

proc print(arr) {
  writeln(arr, "\n: [", arr.domain, "] ", arr.eltType:string);
}

produces:

1.1 1.2
2.1 2.2
: [{1..2, 1..2}] real(64)

However, initializing the same multidimensional array with an array of arrays does not work:

var B: [1..2, 1..2] real = [[1.1, 1.2], [2.1, 2.2]];
print(B);

producing instead:

testit.chpl:2: error: rank mismatch in array assignment

This seems a bit arbitrary and surprising. Why should tuples enjoy more flexibility here than arrays, particularly given that the destination type is an array?

As a result, this issue asks the question, should we extend this support for initialization to arrays?

@damianmoz
Copy link

From a purely selfish perspective (if I can speak for my collegaues), that would address most of the issues we have seen with the consensus prototype of #8864 for most cases of the array literals, probably >95% of the time.

What about the remaining <5%. Some would be for a small array needing to be provided across a parameter list. For that scenario, the _consensus prototype of #8864 would be more than adequate, and I could probably survive that scenario even if that consensus prototype was not available. Addressing what would probably be the remaining 1-2% of use cases can probably be handled by just using an array of arrays.

I need to think a bit more about the answers to Q3 of #8864 in this context but whatever the answer, it would likely be adequate. The concept of an Nx1 or a 1xN 2D array is very much an edge case and if I could not avoid it, I would probably try and cater for it with separate code, especially for N=1. As an off the cuff remark (for whatever that is worth), I cannot see it being an issue. Giving that advice to others for their scenarios? Hmmm? Definitely something I need to think about on a case by case basis. I normally ask my team to avoid Nx1 and 1xN scenarios in their code.

Most of those ideas I presented in #8864 for irregular arrays using the heavyweight comma and parentheses are null and void and we need to go back to square one.. But most of the needs my colleagues might have for an irregular array can be addressed by a tuple of arrays of various shapes. I would find out the number of existing users for which that quick and easy solution does the job in the short term. But irregular arrays are at least off-topic and definitely a whole new ball game and a totally separate issue.

Thanks heaps for raising this.

@damianmoz
Copy link

damianmoz commented Mar 3, 2025

A shorthand cast like

[ 1.0, 2.0, 3.0 ]:real(32)

works in 1D but not beyond 1D. In #8864, I alluded to it being able to be applied to an array of arrays [of arrays ...] It was mentioned ..., "that cast should work in the proposal. At present, the cast would be done at execution-time, but with work, it could potentially be made into a compile-time cast." Should I raise a separate issue? I note that an equivalent cast (but with slightly different syntax) already works perfectly for a tuple of tuples so Chapel already supports the concept in al slightly different context.

I realize I could use the existing way to cast a literal array of arrays of say 27x3 real(64) numbers, as in appending (to the literal definition)

: [1..27][1..3] real(32)

So it is not urgent. But having the same shorthand that works for 1D would be nice.

@bradcray
Copy link
Member Author

bradcray commented Mar 3, 2025

works in 1D but not beyond 1D.

It seems to work for 2D arrays, and I'd expect it to work for nD as well: [ATO]

It doesn't currently work for arrays of arrays, nor do other promotions (e.g., sin(myArrOfArr) because promotion in Chapel is defined on an array-of-t by checking to see whether a subroutine/operator can be resolved for its element type t. If so, the promotion is applied, if not, it's not. I.e., currently promotion doesn't recurse all the way down to the innermost element type. [ATO]

Should I raise a separate issue?

You're welcome to, though you should consider whether to request that promotion of Chapel routines be extended to arrays-of-arrays in general (an intriguing concept, but a potentially heavy lift) or whether you'd be requesting a very specific cast for array-of-array-of…-of-t cases (and if so, what it would be).

@damianmoz
Copy link

To exploit this new behavior of arrays of arrays across the assignment, I figured I needed to be able to declare an array of arrays of real(32) using real literals which are real(64). So I tried to see if this works today.

 const cubeA : [1..27][1..3] real(32) =
 [
        [ -1.0, -1.0, -1.0 ], [ +0.0, -1.0, -1.0 ], [ +1.0, -1.0, -1.0 ],
        [ -1.0, +0.0, -1.0 ], [ +0.0, +0.0, -1.0 ], [ +1.0, +0.0, -1.0 ],
        [ -1.0, +1.0, -1.0 ], [ +0.0, +1.0, -1.0 ], [ +1.0, +1.0, -1.0 ],
        [ -1.0, -1.0, +0.0 ], [ +0.0, -1.0, +0.0 ], [ +1.0, -1.0, +0.0 ],
        [ -1.0, +0.0, +0.0 ], [ +0.0, +0.0, +0.0 ], [ +1.0, +0.0, +0.0 ],
        [ -1.0, +1.0, +0.0 ], [ +0.0, +1.0, +0.0 ], [ +1.0, +1.0, +0.0 ],
        [ -1.0, -1.0, +1.0 ], [ +0.0, -1.0, +1.0 ], [ +1.0, -1.0, +1.0 ],
        [ -1.0, +0.0, +1.0 ], [ +0.0, +0.0, +1.0 ], [ +1.0, +0.0, +1.0 ],
        [ -1.0, +1.0, +1.0 ], [ +0.0, +1.0, +1.0 ], [ +1.0, +1.0, +1.0 ]
]
        : [1..27][1..3] real(32);

No, it fails to compile with

error: illegal cast from [domain(1,int(64),one)] [domain(1,int(64),one)] real(64)
to [domain(1,int(64),one)] [domain(1,int(64),one)] real(32)

I am jumping the gun, in which case, apologies. Or I must be doing something silly. Any hints?

As @bradcray noted the operation is fully supported for a tuple of tuples, the cast being

:(27*(3*real(32)))

@damianmoz

This comment has been minimized.

@bradcray
Copy link
Member Author

bradcray commented Mar 4, 2025

@damianmoz : Unfortunately, you're running into #26808 which @ty1027 pointed out last week—namely, that currently, Chapel doesn't support casting an array expression to another array type. #26809 will fix that issue for array-of-t, but unfortunately doesn't seem to make this array-of-arrays case work in its current form.

@damianmoz
Copy link

Thanks @bradcray. No rush. Thanks @ty1027. I thought I had seen reference to this in the last few days but this but my recall failed (even though it was in front of me).

@bradcray
Copy link
Member Author

bradcray commented Mar 4, 2025

Here's a manual cast operator that could be used for this sort of cast in the meantime. Note that it's very specialized to rectangular arrays-of-arrays (of precisely depth 2):

operator :(src: [?srcD] ?t, type t2) where isArray(t) && srcD.rank == 1 {
  var dst: t2;

  if checkSizes then
    if dst.size != src.size then
      halt("Size mismatch between array argument and result type: ",
           dst.size, " != ", src.size);

  for (i,j) in zip(dst.domain, srcD) {
    if checkSizes then
      if dst[i].size != src[j].size then
        halt("Size mismatch between inner array sizes");
    dst[i] = src[j]: dst[i].eltType;
  }

  return dst;
}

Try it on ATO

@bradcray
Copy link
Member Author

bradcray commented Mar 5, 2025

@damianmoz : I've extended the original array-cast that just got added in #26809 today to include support for casting one array-of-array value to another array-of-array type in #26841. I'm optimistic about getting it into this month's 2.4 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants