Skip to content

fix(iter): map-left/map-right iterate the named operand#255

Merged
singaraiona merged 1 commit into
masterfrom
fix/map-left-right-broadcast
Jun 13, 2026
Merged

fix(iter): map-left/map-right iterate the named operand#255
singaraiona merged 1 commit into
masterfrom
fix/map-left-right-broadcast

Conversation

@singaraiona

Copy link
Copy Markdown
Collaborator

Problem

map-left/map-right were 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:

(map-right (fn [x y] (println "x=% y=%" x y)) 10 [1 2 3])   ;; gave one call, expected three
(map-right concat 10 [1 2 3])                                ;; gave [10 1 2 3], expected ([10 1] [10 2] [10 3])

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 fn is 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 through call_fn2, which already broadcasts atomic builtins over a held vector and hands lambdas their arguments whole.

(map-right (fn [x y] (list x y)) 10 [1 2 3])  → ([10 1] [10 2] [10 3])
(map-right concat 10 [1 2 3])                  → ([10 1] [10 2] [10 3])
(map-left  + 10 [1 2 3])                       → [11 12 13]
(map-right - [10 20 30] 5)                     → [5 15 25]

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 old map-left; that role is now map-right, so the join tests and docs are migrated map-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 (a concat / list-building fn exposes the iteration structure that arithmetic builtins' broadcast otherwise hides).

Tests

Full suite green: 3443/3445 passed (2 skipped, 0 failed).

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).
@singaraiona singaraiona merged commit 4feab81 into master Jun 13, 2026
4 checks passed
@hetoku hetoku deleted the fix/map-left-right-broadcast branch June 13, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant