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

Make c_array interface more like Chapel arrays #17946

Open
stonea opened this issue Jun 18, 2021 · 7 comments
Open

Make c_array interface more like Chapel arrays #17946

stonea opened this issue Jun 18, 2021 · 7 comments

Comments

@stonea
Copy link
Contributor

stonea commented Jun 18, 2021

This is a spinoff from some comments on https://github.com/Cray/chapel-private/issues/2126 :

It would be nice if the following worked:

var a : c_array(c_float, N); 
a = 1..N;

Currently it produces this error:

streamChpl.chpl:17: error: could not find a copy initializer ('init=') for type 'c_array(real(32),10)' from type 'range(int(64),bounded,false)'
$CHPL_HOME/modules/standard/CPtr.chpl:208: note: this candidate did not match: c_array.init=(other: c_array)
streamChpl.chpl:17: note: because actual argument #1 with type 'range(int(64),bounded,false)'
$CHPL_HOME/modules/standard/CPtr.chpl:208: note: is passed to formal 'other: c_array'

I got the following feedback from Brad (see the private Github issue for more details):

I suspect that we could support this easily by adding more assignment overloads for c_arrays. A question I'd want to wrestle with here is whether our goal with c_arrays is to make them only support the operations that such arrays support in C; or whether the goal is to make them support as much of Chapel's array interface as is appropriate (e.g., should I be able to write myCArray[3..] to refer to a slice of the C array? Or should that kind of nicety only be supported for Chapel arrays? (where maybe we'd have a way to "view" a C array as a Chapel array?)). I could probably argue either side of this issue and am not sure where I'd land offhand.

I'd argue that we should make c_array as convenient as possible. Does anyone else have thoughts?

@mppf
Copy link
Member

mppf commented Jun 18, 2021

Supporting this specifically var a : c_array(c_float, N); a = 1..N; is easy and I don't think would cause any major problems down the line. It would just amount to adding an init= and = for c_array from the range type. We would probably want to do this for a few other types as well (such as regular arrays).

Does the converse operation work?

var ca: c_array(int, 10);
var A: [0..<10] int = ca;

I would guess that it does not but we could get it to work if we like.

e.g., should I be able to write myCArray[3..] to refer to a slice of the C array?

I think supporting slicing on c_array is a whole other can of worms and I would lean against it.

@bradcray
Copy link
Member

It would just amount to adding an init= and = for c_array from the range type.

More generally, we may want to have overloads that take _iteratorRecord and other iterable things as well (eventually, something we'll hopefully be able to express using an : Iterable interface constraint).

The philosophical discussion this issue brings up for me is "What is the role/goal of the c_array type?" (to help weigh what things it should or should not support). Is it to support things a C array does and nothing else? Is it to be as much like a Chapel array as possible? Something in-between? In what situations would/should a user use c_array rather than a 1D Chapel array (maybe with a special "in-place" domain map)?

@stonea
Copy link
Contributor Author

stonea commented Jun 18, 2021

Does the converse operation work?

var ca: c_array(int, 10);
var A: [0..<10] int = ca;

Looks like no, I get this error:

foo.chpl:4: error: cannot iterate over values of type c_array(int(64),10)

@mppf
Copy link
Member

mppf commented Jun 18, 2021

In what situations would/should a user use c_array rather than a 1D Chapel array (maybe with a special "in-place" domain map)?

Well for one thing c_array must have a param / compile time size, and as a result is probably more closely related to a tuple than a Chapel array.

@bradcray
Copy link
Member

To be clear, I'm not trying to say "they're the same, why do we have both?", just trying to probe the question of what guidance we give for using them to see if it helps determine "these operations/conveniences should be supported" vs. "these should not."

@stonea
Copy link
Contributor Author

stonea commented Apr 21, 2022

Hi @bradcray. I'm going through and looking at old issues I'm a participant on (which brought me here and to a related issue I have on my private tracker).

I want to ask if you feel addressing this design issue (or some aspect of it) is on the critical path for "Chapel 2.0 or not (I suspect the answer is "no" but I wanted to get a second opinion).

To refresh you, the critical question raised by this issue is the question: "to what extent should the c_array interface match the Chapel array interface".

My take is that going forward we might add to the existing c_array interface (making it more like the Chapel array interface) but it's unlikely that'd we'd remove any of the existing functionality. Since we're not removing anything it's not necessary to stabilize this for Chapel 2.0.

For reference the existing interface for c_array is here:
https://chapel-lang.org/docs/modules/standard/CTypes.html#CTypes.c_array
and the interface for Chapel arrays is here: https://chapel-lang.org/docs/main/language/spec/arrays.html#ChapelArray.array

The only thing I see in c_array that's not in the Chapel array interface is the the writeThis method and keeping that around seems innocuous.

Anyway, let me know if you agree that this is not Chapel 2.0 relevant. If you think it is my next step is figuring out how to bring this to the attention of the Chapel 2.0 team.

@bradcray
Copy link
Member

I agree that it seems likely we could choose to make c_array more capable after Chapel 2.0 without breaking existing codes and that the more important question for 2.0 is whether it supports anything today that we expect to remove from it in the future. So I don't think of this as being on the critical path for 2.0.

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

3 participants