fix(iter): map-left/map-right iterate the named operand#255
Merged
Conversation
map-left and map-right were defined by which argument is held fixed, the inverse of the conventional model and the source of the long-standing confusion (a scalar in the 'wrong' slot, or a lambda that does not broadcast, gave surprising results). Redefine them by which operand is ITERATED: map-left — iterate the left operand, hold the right whole → fn(left_i, right) map-right — iterate the right operand, hold the left whole → fn(left, right_i) The iterated side is fixed by the operator (no auto-detect heuristic). When that side is an atom there is nothing to iterate, so fn is applied once to (left, right); for an atomic builtin that single call is exactly its broadcast, so map-left/map-right agree with plain application there. Every call routes through call_fn2, so atomic builtins broadcast a held vector element-wise and lambdas receive their arguments whole. This swaps the prior meaning of the two ops. The window-join interval idiom built its N per-row [lo hi] intervals with the old map-left; that role is now map-right, so the join tests/docs are migrated map-left -> map-right. Updates the map-left/map-right reference docs and adds non-broadcasting lambda regression tests (a single concat / list-building fn exposes the iteration structure that arithmetic builtins' broadcast otherwise hides).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
map-left/map-rightwere defined by which argument is held fixed, the inverse of the conventional model. This caused surprising results whenever the scalar sat in the "wrong" slot or the function was a non-broadcasting lambda:A prior attempt removed an "auto-detect" heuristic, which only masked the issue because the test suite exercised the cases where atomic builtins broadcast either way.
Fix
Define the ops by which operand is iterated:
map-left— iterate the left, hold the right whole →fn(left_i, right)map-right— iterate the right, hold the left whole →fn(left, right_i)The iterated side is fixed by the operator (no auto-detect). When that side is an atom there's nothing to iterate, so
fnis applied once to(left, right)— and for an atomic builtin that single call is its broadcast, so map-left/map-right agree with plain application there. Every call routes throughcall_fn2, which already broadcasts atomic builtins over a held vector and hands lambdas their arguments whole.Migration
This swaps the prior meaning of the two ops. The window-join interval idiom built its N per-row
[lo hi]intervals with the oldmap-left; that role is nowmap-right, so the join tests and docs are migratedmap-left → map-right(verified to produce identical window-join results). Reference docs for the two ops are updated, and non-broadcasting lambda regression tests were added (aconcat/ list-building fn exposes the iteration structure that arithmetic builtins' broadcast otherwise hides).Tests
Full suite green: 3443/3445 passed (2 skipped, 0 failed).