chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.78k stars 420 forks source link

Support multidimensional arrays with `writeBinary()`? #23081

Closed bradcray closed 1 year ago

bradcray commented 1 year ago

While reviewing @benharsh's recent updates to the shootout benchmarks to make use of new IO features, @ronawho noted that it seemed odd that most of the shootouts could use writeBinary() while mandelbrot had to use the BinarySerializer(). The reason for this is that writeBinary() only accepts 1D DR arrays today, not nD. This seemed a bit arbitrary and like an unfortunate asymmetry in the suite of codes, and we postulated that the implementation would probably be virtually identical for the nD case (take a c_ptrTo() the array, query its size, blast it out to C).

To that end, this issue asks whether we could consider extending writeBinary() to accept nD DR arrays (stored in row-major order).

bradcray commented 1 year ago

@mppf: I have a feeling that the reason for the current 1D-only behavior is that you were opposed to supporting arrays at all, but opened the door to 1D as a concession to me and/or Robert. How opposed are you to extending it to nD (local, row-major order) arrays?

mppf commented 1 year ago

I don't remember what I was worried about, but it does not worry me right now. (Maybe just the amount of duplicate functionality in the I/O module). I think the main challenge to nD beyond what we have now is that we will be defining it to always output in row-major. That seems like something we can handle OK by documenting it.

benharsh commented 1 year ago

Another reason to support this is that calling writeBinary with an nD array can result in promotion over the overload of writeBinary that accepts numeric args. If we decide not to support this, we should probably add an overload that at least issues a compiler error so that users aren't surprised when their IO call is inexplicably parallel.