Add generator-comprehension syntax for Monoid - #727
Open
eb8680 wants to merge 1 commit into
Open
Conversation
eb8680
marked this pull request as ready for review
July 28, 2026 15:25
eb8680
marked this pull request as draft
July 28, 2026 15:26
eb8680
force-pushed
the
eb-comprehension
branch
2 times, most recently
from
July 28, 2026 15:37
4de7e7e to
ad39d08
Compare
eb8680
marked this pull request as ready for review
July 29, 2026 00:07
`Sum(f(x) * g(x, y) for x in xs for y in ys(x))` now desugars to
`Sum.reduce(f(x()) * g(x(), y()), {x: xs, y: ys(x())})` with fresh
Operations standing for an element of each stream.
`effectful/internals/comprehension.py` recovers the comprehension's syntax
via the bytecode disassembler and rebuilds it as a reduction.
Also fixes `ReduceDisequalityMask`, which used `or` to pick the first
non-None of two `_neq_to_plus` results and so asked a `Term` for its
truthiness.
The `_jax_args` fix these tests depend on is no longer duplicated here; it
comes from the base branch `eb-jax-scalar-plus`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Gives
Monoida comprehension syntax, so a loop nest can be written the way you'd write it in Python and still be reduced symbolically.desugars to
where
xandyare freshOperations standing for "an element ofxs" and "an element ofys(x())".How it works
Monoid.__call__hands the generator object toeffectful/internals/comprehension.py, which recovers the comprehension's syntax with the disassembler from the base PR and replays it against the monoid.The interesting part is the typing, not the rewriting. Each loop target becomes an
Operationwhose return type is the element type of its stream, and that type has to be known before the target can be applied to anything in the body. Streams may also depend on earlier targets, asys(x())does, so element types are inferred one generator at a time, left to right: evaluate a stream, infer its element type, mint the target operation, bind it, then move on.Some of what a comprehension may contain is syntax rather than operations.
and,or,notand conditional expressions all ask their operands for a concretebool, which aTermcannot supply, so they are rewritten intoite, which yields one of its arms outright when the condition is concrete and a term when it is not. Comparisons are left alone: numeric terms already implement==and<symbolically.Changes outside the two new files
Three small, independent hunks, each reviewable on its own:
Monoid.__call__(effectful/ops/monoid.py) — the entry point, with a doctest. This is the only part of the diff that is the feature itself.ReduceDisequalityMask(effectful/ops/monoid.py) — an independent bugfix. It usedorto pick the first non-Noneof two_neq_to_plusresults, which asks aTermfor its truthiness. Replaced with an explicitis Nonecheck, which is what the followingif ret is not Nonealready expected.— moved to Don't route all-scalar monoid ops through jax #729, which_jax_args(effectful/handlers/jax/monoid.py)this branch is now stacked behind. These tests depend on it:
jax.typing.ArrayLikeis a union that includes
bool/int/float/complex, and the JAX handlers extendthe global
EvaluateIntpat import time, so once anything importedeffectful.handlers.jax.monoida scalar comprehension body was silently narrowed toa
float32array. Six tests here (including two doctests) failed in a full-suite runbut passed in isolation, purely on import order.
Nothing else from
effectful/ops/monoid.pyin #724 is included here — noReduceGroundCartesianProduct, no inversion rewrite, noReducePartial.unrolled. Those are separate branches.Tests
tests/test_internals_comprehension.py: 734 passed, 5 xfailed.The 5 xfails come from #724 as written and are not waiting on any sibling PR: a generator expression used as an inner stream (
for y in (z for z in range(x))) raisesNotImplementedError, markedstrict=True. A list comprehension in the same position (for y in [z for z in range(2)]) does work and is tested.Full suite (
effectful/ tests/, excluding the LLM handler tests): 19704 passed, 2 skipped, 2085 xfailed, no failures. Doctests are on repo-wide via--doctest-modules, and the newMonoid.__call__doctest passes both standalone and in a full run.ruff checkandruff format --diffare clean.mypyreports one pre-existing error ineffectful/handlers/jax/monoid.py, present onstaging-weightedand unrelated.Split out of #724 for review. Stacked on #725, which is stacked on #729.
Review #729 → #725 → #727 in that order.
🤖 Generated with Claude Code