zippered forall loops with size mismatches can silently drop iterations on the floor #11428
Comments
bradcray
changed the title
1 task
bradcray
added a commit
that referenced
this issue
May 17, 2019
Oops, I had promotion case wrong in this test... [reviewed by @ronawho] I was accidentally using A rather than AS as intended and getting away with it due to the longstanding "forall doesn't check size matches" bug (#11428). This resulted in a ridiculous amount of communication which didn't trigger as a problem as loudly as it should've for me yesterday.
bradcray
added a commit
to bradcray/chapel
that referenced
this issue
Jun 17, 2021
Issue chapel-lang#11428 captures a longstanding issue in which we zipper a smaller thing and a larger one: var A3: [1..3] real; var A4: [1..4] real; forall (i,j) in zip(A3, A4) do .. which is a longstanding challenge in Chapel's current leader-follower iterator design. However, it seems that at some point we started permitting the opposite case through as well: forall (i,j) in zip(A4, A3) do ... which doesn't really make any sense since A3 doesn't have 3 items to yield, so should give an OOB error at some point when trying to do what the leader says. Yet... it doesn't. This is an attempt to fix that by putting bounds checking into the follower iterator. --- Signed-off-by: Brad Chamberlain <bradcray@users.noreply.github.com>
This was referenced Jun 17, 2021
bradcray
added a commit
that referenced
this issue
Jun 18, 2021
Add an error for zippered follower iterations over too-small ranges [reviewed by @e-kayrakli] This adds an error for zippered forall loops in which the follower is a range that does not have enough elements in it. For example: ```chapel forall (i,j) in zip(1..4, 1..3) do ... ``` This is a case that was already generating an error, but not a very clear one, and represents a stepping stone toward generating similar such errors for domains and arrays (which currently quietly pass today, as I was learning last night). This does not address the longstanding and much more complex to fix case when the follower is larger than the leader. I.e., my most dreaded Chapel bug (TM): #11428 I found that there was a previous attempt to create a similar error that currently isn't being testing, and doesn't seem sound to me (in that it compared the length of what a specific follower was being asked to yield to the total range length). This new version checks that the indices being iterated over are within the size bounds of the range to avoid bad orderToIndex() calls (which were resulting in the old, less useful error).
bradcray
added a commit
that referenced
this issue
Jun 19, 2021
Issue an error when a DR domain/array is following a larger leader [reviewed by @vasslitvinov and @stonea] Nikhil shared a case on chat this week where—to my surprise—we weren't generating OOB errors for having a DR array get indexed out of bounds when it was following a larger leader array. This is something that we used to catch, but no longer seem to. Here, I'm adding a check to the DR domain follower code to catch such cases and a new test to lock the check in. Note that this doesn't address the longstanding error in which a leader iterator expression is smaller than its follower (i.e., my most dreaded Chapel bug (TM): #11428).
This was referenced Nov 16, 2022
Open
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
[This predates our use of GitHub issues, but seems never to have been captured here?]
When doing zippered loops over multiple iterands of mismatched sizes where the first expression is smaller than subsequent ones, the compiler drops the extra iterations on the floor rather than complaining about a size mismatch. For example, this occurs for the following simple case:
(Potentially too much detail for non-developers): The reason for this is that our current leader-follower interface asks each follower to iterate over a specific subset of the leader's iteration space, but there's no point at which the global iteration space is available to the follower iterator(s) to notice that some of their items will not be requested. At times, we've proposed that a "leader follower 2.0" interface might include an initial (or teardown in the case of an iterator expression of unpredictable size?) dialogue between the leader and follower expressions to make sure that size checks are reasonable. Alternatively, we could update the follower interface to take not just the set of indices to yield, but also the complete set of indices such that it could do a size check prior to starting its local iteration.
Associated Future Test(s):
test/functions/iterators/bradc/badzip/arrayZipMismatch.chplConfiguration Information
chpl --version:chpl version 1.19.0 pre-release (04f5ef4)(and since time immemorial)keywords: array size mismatch zip zipped
The text was updated successfully, but these errors were encountered: