chapel-lang / chapel

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

meaning of `+` `-` `|` `&` `^` on domains and ranges #19254

Open vasslitvinov opened 2 years ago

vasslitvinov commented 2 years ago

This is a summary and generalization of #17101. It proposes a user view of these operations that is simple and uniform. related: * / % in #19264

Summary

There are several ways that each of these operations can be interpreted. We favor keeping one of these interpretation -- set operations on irregular domains -- and disabling all the other interpretations with compiler errors.

This gives us the flexibility to respond to future user requests to enable other interpretation(s) -- while remaining backwards compatible.

Also, the implementation strategy proposed here allows user code to define and use other interpretations as desired.

Possible interpretations

These operations, when applied to one or two domains (similarly for range(s) and combinations), could be interpreted as:

Discussion

For + and - there can be a confusion whether they mean a set operation, which is valid only on irregular domains, or translation of the index set, which we have done only for rectangular domains and ranges. We would like to eliminate such confusion by allowing the former and disallowing the latter. See, for example, https://github.com/chapel-lang/chapel/issues/17101#issuecomment-775155915 and https://github.com/chapel-lang/chapel/issues/17101#issuecomment-859156659 .

We also do not want to leave room for confusion between the set interpretation and promotion https://github.com/chapel-lang/chapel/issues/17101#issuecomment-776708194 and dislike their promoted interpretation https://github.com/chapel-lang/chapel/issues/17101#issuecomment-859154057 .

As an anecdote, (a two-dimensional rectangular domain) + (an integer) resolves to a promoted + on a 2-tuple and integer, producing a stream of shifted indices. While this is a valid interpretation, it is more likely not what the user intended.

As another anecdote, (an associative domain of integers) + (a range) produces a stream of newly-created associative domains that result by adding each index of the range to the original domain. This also is a valid yet unlikely interpretation. This proposal makes this expression a compilation error.

This leads us to disallowing these operations in all cases except as set operations on irregular domains.

Notably, this takes away the closed-form algebra of transformations, whereby (a range) + (an integer) would still be still a range. This sounds acceptable to us, given that the closed-form algebra can be achieved using named functions like translate. // Its specifics are discussed in #17127 .

Implementation proposal

This proposal is to provide the following three overloads for each operation:

inline operator +(in dom: domain, other) { ... }
inline operator +(other, in dom: domain) { ... }
inline operator +(in dom1: domain, dom2: domain) { ... }

Each overload contains param conditionals that check the types of dom and other and invoke the desired implementation in each case.

Cf. currently each operation contains overloads catering to specific implementations, ex. one overload for (rectangular domain) + (index), another for (irregular domain) + (index), etc.

The primary benefit is that we can easily issue easy-to-understand "not allowed" messages in all the cases that are not allowed.

The secondary benefit is that it is easier for users to overload more specific cases. For example, if I want to make my stencil computations look pretty like in https://github.com/chapel-lang/chapel/issues/17101#issuecomment-774342834 :

var B = A[D+north] + A[D+south] + A[D+east] + A[D+west];

I can define operator +(in dom: domain, dir: 2*int) return dom.translate(dir); perhaps with a where-clause. The resolution will prefer it in my code over the predefined overloads.

One big downside of this implementation style in general is that I do not get a promotion of an allowed case. Instead resolution will match my wannabe-promoted actual against the fully-generic formal and likely issue the error "this case is not allowed". However, an error message is exactly what we want under this proposal, given that we are disallowing promotion interpretations.

Clarification

This proposal does not outlaw the underlying operations or promotion. It simply requires the user to be explicit about what they mean:

vasslitvinov commented 2 years ago

During today's domain module re-review, we generally liked this proposal and upheld the goal of having domains and ranges behave the same way whenever applicable.

We also wanted to entertain this alternative proposal: do all the same, except replace "allow only the set interpretation with irregular domains" with "allow only the translation interpretation with all domains".

Specifically:

The key benefit is that this gives us something very close to promotion, except here we get a compact representation. Promotion is what we'd get without treating these cases specially, and also what we get for these operations on arrays. So this alternative proposal offers something close to consistency.

Open questions with this alternative proposal:

mppf commented 2 years ago

for dom + idx where dom is an irregular domain, should the outcome be:

IMO compilation error, since otherwise it would mean "translate" and "translate" doesn't make sense for irregular.

how to treat | & ^ ?

IMO leave them to be promoted unless we decide to make errors for * / % on domains

vasslitvinov commented 2 years ago

Reporting on my experimentation.

Experiments for + - | & ^ * / % op=

I ran a full paratest on each of the following changes:

Findings for + - | & ^ in the non-op= experiment

The only failed test that is intended as user-like code is release/examples/primers/associative, plus its dyno counterpart. It goes like this:

// test/release/examples/primers/associative.chpl
var primeDom = {2, 3, 5, 7, 11, 13, 17};  // some prime numbers
var fibDom   = {0, 1, 1, 2, 3, 5, 8, 13}; // part of the Fibonacci sequence
var primeAndFib = primeDom & fibDom;
writeln("Some primes in the Fibonacci sequence: ", primeAndFib);
writeln("Some primes not in the Fibonacci sequence: ", primeDom - primeAndFib);

var Names: domain(string);
Names += "some name";
Name += "another name";
.....
Name -= "an existing name";
.....
var Women = {"Alice", "Dana", "Ellen"};
var Men = Names - Women;
assert( (Men | Women) == Names );

All in all, 33 tests failed. A lot of them failed because they invoked an op= which is implemented via the corresponding op that now reports an error. A handful of the failures are not significant for this discussion, like due to a different number of resolution candidates in visibility/ tests.

list of failing tests ``` associative/bharshbarg/domains/domainSetOps associative/bharshbarg/domains/opEquals associative/bharshbarg/domains/reduceArrOfAssocDom associative/ferguson/plus-reduce-assoc associative/ferguson/plus-reduce-intent-assoc deprecated/domain-or-and-xor domains/compilerErrors/rect-and-eq domains/compilerErrors/rect-or-eq domains/compilerErrors/rect-xor-eq domains/diten/assocDomIntersect domains/sungeun/assoc/plus_equals dyno/primers/associative exercises/duplicates/duplicates-alt-assoc-reduce expressions/bradc/hashVsBinaryPrec expressions/if-expr/if-loop-const.bad io/ferguson/big-precision-t-lossless library/standard/LinkedLists/bradc/multArithSeq reductions/vass/histogram-1 reductions/vass/histogram-2 release/examples/primers/associative types/range/bradc/rangeMath2 types/range/deitz/test_range_iterate_error2 types/tuple/sungeun/homTupleOfDomainsTypeAlias users/engin/sparse_bulk_dist/sparseAdd1D users/engin/sparse_bulk_dist/sparseAdd2D users/engin/sparse_bulk_dist/sparseBulk1D users/engin/sparse_bulk_dist/sparseBulk2D visibility/except/operatorsExceptions/exceptAnd visibility/except/operatorsExceptions/exceptDivision visibility/except/operatorsExceptions/exceptMod visibility/except/operatorsExceptions/exceptMultiplication visibility/except/operatorsExceptions/exceptOr visibility/except/operatorsExceptions/exceptXor ```

Findings for += -= |= &= ^= in the op= experiment

Overall: ~400 tests use at least one of these on domains/ranges. Of these:

Disclaimer: my experiment was simplistic as it counted only a single op= per (test,compopt) and included "uninteresting" tests that verify existing compiler errors for these op=. So the counts here are approximate.

Let us focus on "user-like" codes, as opposed to code written for testing purposes.

list of "user-like" tests ``` exercises/duplicates/duplicates-alt-assoc-reduce.chpl exercises/duplicates/duplicates-alt-assoc.chpl exercises/duplicates/duplicates-bonus-b2-record.chpl exercises/duplicates/duplicates-bonus-b3-size.chpl exercises/duplicates/duplicates-step0-start.chpl exercises/duplicates/duplicates-step1-for.chpl exercises/duplicates/duplicates-step2-forall.chpl exercises/duplicates/duplicates-step3-error.chpl library/packages/MatrixMarket/mm-read-sparse.chpl library/packages/UnitTest/Launcher_Test.chpl mason/build/noDeps.chpl mason/chplVersion/mason-chpl-version-update.chpl mason/mason-example-with-opts/mason-example.chpl mason/mason-example/mason-example.chpl mason/mason-external/libtomlc99/mason-external.chpl mason/mason-external/masonExternalRanges/mason-external-range.chpl mason/mason-test/mason-test.chpl mason/mason-update-toml-error/mason-chpl-version-update.chpl mason/masonTestSome/mason-test-some.chpl mason/masonTestSubString/mason-test-sub-string.chpl mason/masonUpdateTest.chpl mason/pkgconfig-tests/openssl.chpl mason/pkgconfig-tests/zlib.chpl mason/run/mason-run.chpl mason/subdir-commands/mason-run.chpl npb/cg/bradc/cg-dense.chpl npb/cg/bradc/cg-makea1-small-sparse-countInds.chpl npb/cg/bradc/cg-makea1-small-sparse.chpl npb/cg/bradc/cg-makea1-small.chpl npb/cg/bradc/cg-makea1-sparse-sort.chpl npb/cg/bradc/cg-makea1.chpl npb/cg/bradc/cg-makea2-small-sparse.chpl npb/cg/bradc/cg-makea2.chpl npb/cg/bradc/cg-makea3.chpl npb/cg/bradc/cg-printa.chpl npb/cg/bradc/cg-sparse-partred.chpl npb/cg/bradc/cg-sparse.chpl puzzles/deitz/digits3.chpl puzzles/deitz/roman.chpl reductions/userdefined/slowSum.chpl release/examples/benchmarks/lulesh/lulesh.chpl release/examples/benchmarks/lulesh/test3DLulesh.chpl release/examples/primers/LinearAlgebralib.chpl release/examples/primers/associative.chpl release/examples/primers/domains.chpl release/examples/primers/learnChapelInYMinutes.chpl release/examples/primers/sparse.chpl release/examples/primers/specialMethods.chpl release/examples/spec/Records/userhash.chpl studies/590o/blerner/xml-parse.chpl studies/590o/bradc/coforall-private-indices-workaround.chpl studies/590o/bradc/coforall-private-indices.chpl studies/590o/bradc/forall-begin-private-indices.chpl studies/590o/bradc/intset.chpl studies/590o/kfm/solver-blc.chpl studies/590o/kfm/solver.chpl studies/adventOfCode/2021/bradc/day12.chpl studies/adventOfCode/2021/bradc/day12a.chpl studies/adventOfCode/2021/bradc/day15.chpl studies/adventOfCode/2021/bradc/day15a.chpl studies/adventOfCode/2021/bradc/day8a.chpl studies/bale/toposort/toposort.chpl studies/glob/test_glob.chpl studies/labelprop/labelprop-tweets.chpl studies/lsms/shemmy/m-lsms.chpl studies/lsms/shemmy/s-lsms.chpl studies/lulesh/bradc/lulesh-dense.chpl studies/lulesh/bradc/xyztuple/lulesh-dense-3tuple.chpl studies/twitter/strshuffle.chpl ```

Of these 69 tests:

Conclusions

vasslitvinov commented 1 year ago

We are considering these unstable for 2.0. I removed the 2.0 label.