Skip to content

Commit

Permalink
Range domain deprecations (chapel-lang#22823)
Browse files Browse the repository at this point in the history 
Deprecate the following:

- range.boundsCheck(idx) -> range.contains(idx)
  range.boundsCheck(range2) --> no replacement publicly
  range.boundsCheck(range2) --> chpl_boundsCheck() internally

chapel-lang#17125 (comment)

- domain.dist --> .distribution

Cray/chapel-private#4886 (comment)

- assignment between unbounded ranges
  chapel-lang#22559

Also fixes a previously untested bug in `ChapelRange` where
`isFiniteIdxType`
was misspelled.

[Reviewed by @vasslitvinov ]
  • Loading branch information
@ShreyasKhandekar
ShreyasKhandekar authored Jul 28, 2023
2 parents 16cfdb8 + 2271797 commit 8a77d58
Show file tree
Hide file tree
Showing 66 changed files with 290 additions and 279 deletions.
2 changes: 1 addition & 1 deletion modules/dists/BlockDist.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1208,7 +1208,7 @@ proc BlockArr.dsiDynamicFastFollowCheck(lead: []) do

proc BlockArr.dsiDynamicFastFollowCheck(lead: domain) {
// TODO: Should this return true for domains with the same shape?
return lead.dist.dsiEqualDMaps(this.dom.dist) && lead._value.whole == this.dom.whole;
return lead.distribution.dsiEqualDMaps(this.dom.dist) && lead._value.whole == this.dom.whole;
}

iter BlockArr.these(param tag: iterKind, followThis, param fast: bool = false) ref where tag == iterKind.follower {
Expand Down
2 changes: 1 addition & 1 deletion modules/dists/CyclicDist.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ proc CyclicArr.dsiDynamicFastFollowCheck(lead: []) do
return this.dsiDynamicFastFollowCheck(lead.domain);

proc CyclicArr.dsiDynamicFastFollowCheck(lead: domain) {
return lead.dist.dsiEqualDMaps(this.dom.dist) && lead._value.whole == this.dom.whole;
return lead.distribution.dsiEqualDMaps(this.dom.dist) && lead._value.whole == this.dom.whole;
}

iter CyclicArr.these(param tag: iterKind, followThis, param fast: bool = false) ref where tag == iterKind.follower {
Expand Down
2 changes: 1 addition & 1 deletion modules/dists/StencilDist.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1295,7 +1295,7 @@ proc StencilArr.dsiDynamicFastFollowCheck(lead: []) do
return this.dsiDynamicFastFollowCheck(lead.domain);

proc StencilArr.dsiDynamicFastFollowCheck(lead: domain) {
return lead.dist.dsiEqualDMaps(this.dom.dist) && lead._value.whole == this.dom.whole;
return lead.distribution.dsiEqualDMaps(this.dom.dist) && lead._value.whole == this.dom.whole;
}

iter StencilArr.these(param tag: iterKind, followThis, param fast: bool = false) ref where tag == iterKind.follower {
Expand Down
8 changes: 4 additions & 4 deletions modules/internal/BytesStringCommon.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ module BytesStringCommon {
// byteIndex
const intR = r:range(int, r.bounds, r.strides);
if boundsChecking {
if !x.byteIndices.boundsCheck(intR) {
if !x.byteIndices.chpl_boundsCheck(intR) {
halt("range ", r, " out of bounds for " + t:string + " with length ",
x.numBytes);
}
Expand All @@ -394,7 +394,7 @@ module BytesStringCommon {
// if the low bound of the range is within the byteIndices of the
// string, it must be the initial byte of a codepoint
if r.hasLowBound() &&
x.byteIndices.boundsCheck(r.lowBound:int) &&
x.byteIndices.contains(r.lowBound:int) &&
!isInitialByte(x.byte[r.lowBound:int]) {
throw new CodepointSplitError(
"Byte-based string slice is not aligned to codepoint boundaries. " +
Expand All @@ -403,7 +403,7 @@ module BytesStringCommon {
// if the "high bound of the range plus one" is within the byteIndices
// of the string, that index must be the initial byte of a codepoint
if r.hasHighBound() &&
x.byteIndices.boundsCheck(r.highBound:int+1) &&
x.byteIndices.contains(r.highBound:int+1) &&
!isInitialByte(x.byte[r.highBound:int+1]) {
throw new CodepointSplitError(
"Byte-based string slice is not aligned to codepoint boundaries. " +
Expand Down Expand Up @@ -431,7 +431,7 @@ module BytesStringCommon {
// codepointIdx
const intR = r:range(int, r.bounds, r.strides);
if boundsChecking {
if !x.indices.boundsCheck(intR) {
if !x.indices.chpl_boundsCheck(intR) {
halt("range ", r, " out of bounds for string with length ", x.size);
}
}
Expand Down
28 changes: 14 additions & 14 deletions modules/internal/ChapelArray.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ module ChapelArray {
}
var dimstr = "";
for param i in 0..rank-1 {
if !value.dom.dsiDim(i).boundsCheck(indices(i)) {
if !value.dom.dsiDim(i).contains(indices(i)) {
if dimstr == "" {
dimstr = "out of bounds in dimension " + i:string +
" because index " + indices(i):string +
Expand Down Expand Up @@ -1061,7 +1061,7 @@ module ChapelArray {
if this.isRectangular() {
var ok = true;
for param i in 0..rank-1 {
ok &&= value.dom.dsiDim(i).boundsCheck(ranges(i));
ok &&= value.dom.dsiDim(i).chpl_boundsCheck(ranges(i));
}
if ok == false {
if rank == 1 {
Expand All @@ -1081,7 +1081,7 @@ module ChapelArray {
}
var dimstr = "";
for param i in 0..rank-1 {
if !value.dom.dsiDim(i).boundsCheck(ranges(i)) {
if !value.dom.dsiDim(i).chpl_boundsCheck(ranges(i)) {
if dimstr == "" {
dimstr = "out of bounds in dimension " + i:string +
" because slice index " + ranges(i):string +
Expand Down Expand Up @@ -1394,7 +1394,7 @@ module ChapelArray {
@chpldoc.nodoc
proc checkRankChange(args) {
for param i in 0..args.size-1 do
if !_value.dom.dsiDim(i).boundsCheck(args(i)) then
if !_value.dom.dsiDim(i).chpl_boundsCheck(args(i)) then
halt("array slice out of bounds in dimension ", i, ": ", args(i));
}

Expand Down Expand Up @@ -1516,7 +1516,7 @@ module ChapelArray {
// change this in the future when we have better syntax for
// indicating a generic domain map)..
//
if (formalDom.dist._value.type != unmanaged DefaultDist) {
if (formalDom.distribution._value.type != unmanaged DefaultDist) {
//
// First, at compile-time, check that the domain's types are
// the same:
Expand All @@ -1528,10 +1528,10 @@ module ChapelArray {
// Then, at run-time, check that the domain map's values are
// the same (do this only if the runtime checks argument is true).
//
if (runtimeChecks && formalDom.dist != this.domain.dist) then
if (runtimeChecks && formalDom.distribution != this.domain.distribution) then
halt("Domain map mismatch passing array argument:\n",
" Formal domain map is: ", formalDom.dist, "\n",
" Actual domain map is: ", this.domain.dist);
" Formal domain map is: ", formalDom.distribution, "\n",
" Actual domain map is: ", this.domain.distribution);
}

//
Expand Down Expand Up @@ -1608,8 +1608,8 @@ module ChapelArray {
pragma "no auto destroy"
const updom = {(...newDims)};

const redist = new unmanaged ArrayViewReindexDist(downDistPid = this.domain.dist._pid,
downDistInst=this.domain.dist._instance,
const redist = new unmanaged ArrayViewReindexDist(downDistPid = this.domain.distribution._pid,
downDistInst=this.domain.distribution._instance,
updom = updom._value,
downdomPid = dom.pid,
downdomInst = dom);
Expand Down Expand Up @@ -2453,7 +2453,7 @@ module ChapelArray {
}
if isSubtype(eltType, _domain) {
const ref lhsDist = chpl__distributionFromDomainRuntimeType(eltType);
const ref rhsDist = elt.dist;
const ref rhsDist = elt.distribution;
if lhsDist._instance != rhsDist._instance {
runtimeTypesDiffer = true;
}
Expand Down Expand Up @@ -3165,7 +3165,7 @@ module ChapelArray {
proc chpl__initCopy(const ref rhs: domain, definedConst: bool)
where rhs.isRectangular() {

var lhs = new _domain(rhs.dist, rhs.rank, rhs.idxType, rhs.strides,
var lhs = new _domain(rhs.distribution, rhs.rank, rhs.idxType, rhs.strides,
rhs.dims(), definedConst=definedConst);
return lhs;
}
Expand All @@ -3174,7 +3174,7 @@ module ChapelArray {
proc chpl__initCopy(const ref rhs: domain, definedConst: bool)
where rhs.isAssociative() {

var lhs = new _domain(rhs.dist, rhs.idxType, rhs.parSafe,
var lhs = new _domain(rhs.distribution, rhs.idxType, rhs.parSafe,
definedConst=definedConst);
// No need to lock this domain since it's not exposed anywhere yet.
// No need to handle arrays over this domain either for the same reason.
Expand All @@ -3186,7 +3186,7 @@ module ChapelArray {
proc chpl__initCopy(const ref rhs: domain, definedConst: bool)
where rhs.isSparse() {

var lhs = new _domain(rhs.dist, rhs.parentDom, definedConst=definedConst);
var lhs = new _domain(rhs.distribution, rhs.parentDom, definedConst=definedConst);
// No need to lock this domain since it's not exposed anywhere yet.
// No need to handle arrays over this domain either for the same reason.
lhs._instance.dsiAssignDomain(rhs, lhsPrivate=true);
Expand Down
2 changes: 1 addition & 1 deletion modules/internal/ChapelAutoLocalAccess.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ module ChapelAutoLocalAccess {
return true;

// or at least if they were distributed the same way
if accessBase.domain.dist == loopDomain.dist then return true;
if accessBase.domain.distribution == loopDomain.distribution then return true;

// if we are iterating over a rectangular that's:
// 1. not remote
Expand Down
45 changes: 25 additions & 20 deletions modules/internal/ChapelDomain.chpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,18 @@ module ChapelDomain {

proc chpl__convertValueToRuntimeType(dom: domain) type
where isSubtype(dom._value.type, BaseRectangularDom) {
return chpl__buildDomainRuntimeType(dom.dist, dom._value.rank,
return chpl__buildDomainRuntimeType(dom.distribution, dom._value.rank,
dom._value.idxType, dom._value.strides);
}

proc chpl__convertValueToRuntimeType(dom: domain) type
where isSubtype(dom._value.type, BaseAssociativeDom) {
return chpl__buildDomainRuntimeType(dom.dist, dom._value.idxType, dom._value.parSafe);
return chpl__buildDomainRuntimeType(dom.distribution, dom._value.idxType, dom._value.parSafe);
}

proc chpl__convertValueToRuntimeType(dom: domain) type
where isSubtype(dom._value.type, BaseSparseDom) {
return chpl__buildSparseDomainRuntimeType(dom.dist, dom._value.parentDom);
return chpl__buildSparseDomainRuntimeType(dom.distribution, dom._value.parentDom);
}

proc chpl__convertValueToRuntimeType(dom: domain) type {
Expand Down Expand Up @@ -349,7 +349,7 @@ module ChapelDomain {
for param i in 0..dom.rank-1 do
ranges(i) = ranges(i) # counts(i);

return new _domain(dom.dist, dom.rank, dom.idxType, dom.strides, ranges);
return new _domain(dom.distribution, dom.rank, dom.idxType, dom.strides, ranges);
}

@chpldoc.nodoc
Expand Down Expand Up @@ -808,7 +808,7 @@ module ChapelDomain {
var t = _makeIndexTuple(a.rank, b, "step", expand=true);
for param i in 0..a.rank-1 do
r(i) = a.dim(i) by t(i);
return new _domain(a.dist, a.rank, a._value.idxType, newStrides, r);
return new _domain(a.distribution, a.rank, a._value.idxType, newStrides, r);
}

@chpldoc.nodoc
Expand All @@ -818,7 +818,7 @@ module ChapelDomain {
var r: a.rank*range(a._value.idxType, boundKind.both, newStrides);
for param i in 0..a.rank-1 do
r(i) = a.dim(i) by b;
return new _domain(a.dist, a.rank, a._value.idxType, newStrides, r);
return new _domain(a.distribution, a.rank, a._value.idxType, newStrides, r);
}

// This is the definition of the 'align' operator for domains.
Expand All @@ -831,13 +831,13 @@ module ChapelDomain {
var t = _makeIndexTuple(a.rank, b, "alignment", expand=true);
for param i in 0..a.rank-1 do
r(i) = a.dim(i) align t(i);
return new _domain(a.dist, a.rank, a._value.idxType, a.strides, r);
return new _domain(a.distribution, a.rank, a._value.idxType, a.strides, r);
}

// This function exists to avoid communication from computing _value when
// the result is param.
proc domainDistIsLayout(d: domain) param {
return d.dist._value.dsiIsLayout();
return d.distribution._value.dsiIsLayout();
}

pragma "find user line"
Expand Down Expand Up @@ -1084,15 +1084,15 @@ module ChapelDomain {
// handle the type of 'other'. That case is currently managed by the
// compiler and various helper functions involving runtime types.
proc init=(const ref other : domain) where other.isRectangular() {
this.init(other.dist, other.rank, other.idxType, other.strides,
this.init(other.distribution, other.rank, other.idxType, other.strides,
other.dims());
}

proc init=(const ref other : domain) {
if other.isAssociative() {
this.init(other.dist, other.idxType, other.parSafe);
this.init(other.distribution, other.idxType, other.parSafe);
} else if other.isSparse() {
this.init(other.dist, other.parentDom);
this.init(other.distribution, other.parentDom);
} else {
compilerError("cannot initialize '", this.type:string, "' from '", other.type:string, "'");
this.init(nil);
Expand Down Expand Up @@ -1160,8 +1160,13 @@ module ChapelDomain {

/* Return the domain map that implements this domain */
pragma "return not owned"
@deprecated("domain.dist is deprecated, please use domain.distribution instead")
proc dist do return _getDistribution(_value.dist);

/* Return the domain map that implements this domain */
pragma "return not owned"
proc distribution do return _getDistribution(_value.dist);

/* Return the number of dimensions in this domain */
proc rank param {
if this.isRectangular() || this.isSparse() then
Expand Down Expand Up @@ -1297,7 +1302,7 @@ module ChapelDomain {
for param i in 0..rank-1 {
r(i) = myDims(i)[ranges(i)];
}
return new _domain(dist, rank, _value.idxType, r(0).strides, r);
return new _domain(distribution, rank, _value.idxType, r(0).strides, r);
}

// domain rank change
Expand Down Expand Up @@ -1341,8 +1346,8 @@ module ChapelDomain {
upranges(d) = emptyrange;
}

const rcdist = new unmanaged ArrayViewRankChangeDist(downDistPid=dist._pid,
downDistInst=dist._instance,
const rcdist = new unmanaged ArrayViewRankChangeDist(downDistPid=distribution._pid,
downDistInst=distribution._instance,
collapsedDim=collapsedDim,
idx = idx);
// TODO: Should this be set?
Expand Down Expand Up @@ -2306,7 +2311,7 @@ module ChapelDomain {
}
}

return new _domain(dist, rank, _value.idxType, strides, ranges);
return new _domain(distribution, rank, _value.idxType, strides, ranges);
}

/* Return a new domain that is the current domain expanded by
Expand All @@ -2321,7 +2326,7 @@ module ChapelDomain {
var ranges = dims();
for i in 0..rank-1 do
ranges(i) = dim(i).expand(off);
return new _domain(dist, rank, _value.idxType, strides, ranges);
return new _domain(distribution, rank, _value.idxType, strides, ranges);
}

@chpldoc.nodoc
Expand Down Expand Up @@ -2353,7 +2358,7 @@ module ChapelDomain {
var ranges = dims();
for i in 0..rank-1 do
ranges(i) = dim(i).exterior(off(i));
return new _domain(dist, rank, _value.idxType, strides, ranges);
return new _domain(distribution, rank, _value.idxType, strides, ranges);
}

/* Return a new domain that is the exterior portion of the
Expand Down Expand Up @@ -2408,7 +2413,7 @@ module ChapelDomain {
}
ranges(i) = _value.dsiDim(i).interior(off(i));
}
return new _domain(dist, rank, _value.idxType, strides, ranges);
return new _domain(distribution, rank, _value.idxType, strides, ranges);
}

/* Return a new domain that is the interior portion of the
Expand Down Expand Up @@ -2462,7 +2467,7 @@ module ChapelDomain {
var ranges = dims();
for i in 0..rank-1 do
ranges(i) = _value.dsiDim(i).translate(off(i));
return new _domain(dist, rank, _value.idxType, strides, ranges);
return new _domain(distribution, rank, _value.idxType, strides, ranges);
}

/* Return a new domain that is the current domain translated by
Expand Down Expand Up @@ -2493,7 +2498,7 @@ module ChapelDomain {
var ranges = dims();
for i in 0..rank-1 do
ranges(i) = dim(i).chpl__unTranslate(off(i));
return new _domain(dist, rank, _value.idxType, strides, ranges);
return new _domain(distribution, rank, _value.idxType, strides, ranges);
}

@chpldoc.nodoc
Expand Down
Loading

0 comments on commit 8a77d58

Please sign in to comment.