chapel-lang / chapel

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

Should `.localSubdomain` on a distributed sparse domain return a ref to the local domain? #16772

Open bradcray opened 3 years ago

bradcray commented 3 years ago

As an advanced Chapel programmer, while working with distributed sparse domains, I found myself wanting to do operations on its local sparse domain independently of the global whole. I was thinking that I could do this through the .localSubdomain() routine, but since that currently returns a const, it only allowed me to do read-only operations on my local data rather than modifying it. Should we consider relaxing that, or is that too dangerous?

e-kayrakli commented 3 years ago

My immediate reaction is "that's too dangerous".

But I want to understand the use case a bit better. Because things like this came up in the past, and it is the reason we have an addOn optional argument on bulkAdd. If I remember correctly, this was done so as to make the initialization of PRK Sparse (or NAS-CG?) faster by leveraging a priori knowledge on index-to-locale mapping. So, maybe, depending on your use-case, what we want is to expand the sparse interface to allow such operations to be done fast and safely.

Also, implementation-wise, how can we handle that today? I am guessing even though we may want what you're proposing, we want to prevent myDom.localSubdomain() = anotherSparseDom.

bradcray commented 3 years ago

Good point about bulkAdd and addOn. My desire for this feature came from knowing that a block-distributed sparse domain/array is made up of per-locale sparse domains/arrays and having operations that I wanted to do on the local representation. I think the advantage to this approach over things like bulkAdd + addOn is that it feels like a more natural composition of core concepts and features rather than leaning on methods and arguments to those methods, at least for the power programmer.

But in asking the question, I'm truly asking the question, not trying to assert that we should.

e-kayrakli commented 3 years ago

I see. So you are asking whether we should be able to assign to localSubdomain()?

FWIW, you can do that with bulkAdd+addOn like this:

var localParts = // an array of non-distributed sparse subdomains

coforall l in myEmptySparseBlockDom.targetLocales() do on l {
  myEmptySparseBlockDom.bulkAdd(localParts[l.id], addOn=Locales[l.id]);
}

Where if we allow modifications to local subdomain we could instead write:

var localParts = // an array of non-distributed sparse subdomains

coforall l in myEmptySparseBlockDom.targetLocales() do on l {
  myEmptySparseBlockDom.localSubdomain() += localParts[l.id];
}

which doesn't seem like a big improvement to me.

One alternative maybe adding the option to init a sparse domain from array of sparse domains. There'll be some locality checks etc, but we can optimize for well-aligned cases without too much overhead, I am hoping. That would amount to:

var localParts = // an array of non-distributed sparse subdomains
var myEmptySparseBlockDom: sparse subdomain(someBlockDom) = localParts;

I think we have something like this in the sparse interface when the rhs is array of indices. But probably not for array of domains. Though, this is not as general a solution as this issue asks.

bradcray commented 3 years ago

So you are asking whether we should be able to assign to localSubdomain()?

Right. And the context for this question is the SPMD-style pseudocode that I'm working on transcribing to Chapel here.

which doesn't seem like a big improvement to me.

In terms of the amount of code, I agree. I do think it's slightly clearer, but that's not the crux of my argument. Instead, I'm arguing that the notion of querying the local subdomain or subarray of a distributed array or domain is a general operation that's widely applicable (as is +=); whereas the .bulkAdd(addOn=) method is pretty specialized to distributed sparse domains. So the former exercises general features and knowledge that I might have from other areas of Chapel while the latter is more specialized and requires more work to learn about it / find that it exists. It could be argued that I just happen to be more familiar with one than the other, but I'm guessing we won't ever want to support .bulkAdd(addOn=) for dense rectangular domains, for example.

Of course, if localSubdomain() were to return a ref for distributed sparse domains, we wouldn't want it to do so for distributed dense domains, but that seems like it could be achieved by using different return intents for the different cases and make sure the compiler's const-checking is working.

One alternative maybe adding the option to init a sparse domain from array of sparse domains.

This isn't particularly attractive to me since it doesn't really support the SPMD- / local-view style where I expect things like querying local subdomains and subarrays to arise most often.