Add explicit fallback to array reduction; add error value#485
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the EDG IR and both the Python frontend + Scala compiler to (1) make unary set/reduction operations define an explicit result for empty sets/arrays (improving type propagation), and (2) represent compiler/evaluation failures as an error value that can propagate, enabling partial compilation instead of hard failures (resolves #223).
Changes:
- Extend the IR with
ValueLit.error/ErrorLitand propagate errors through the compiler as a first-class value. - Add
empty_valuetoUnarySetExprand update frontend bindings + compiler evaluation/visitors to use explicit empty-set fallbacks. - Rework constant propagation error handling to stop propagation on errors and track over-assignments separately; export parameter errors in compiled design output.
Reviewed changes
Copilot reviewed 32 out of 33 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| proto/edgir/lit.proto | Adds ErrorLit and ValueLit.error oneof arm for error literals. |
| proto/edgir/expr.proto | Adds UnarySetExpr.empty_value to define empty-set results (esp. reductions). |
| edg/hdl_server/main.py | Bumps Python-side proto version constant to 11. |
| edg/edgir/lit_pb2.pyi | Regenerated stubs to include ErrorLit / ValueLit.error. |
| edg/edgir/lit_pb2.py | Regenerated Python protobuf code for new literal types. |
| edg/edgir/expr_pb2.pyi | Regenerated stubs to include UnarySetExpr.empty_value. |
| edg/edgir/expr_pb2.py | Regenerated Python protobuf code for UnarySetExpr.empty_value. |
| edg/edgir/init.py | Adds Python ErrorValue and decodes ValueLit.error to it. |
| edg/core/test_simple_expr_eval.py | Adds test coverage for overassign producing an error value when ignore_errors=True. |
| edg/core/test_link.py | Updates expected constraint protos to include unary_set.empty_value. |
| edg/core/test_compiled_design_export.py | Adds export test ensuring param errors surface in exported JSON model. |
| edg/core/test_block_portvector.py | Updates expected SUM constraint to include empty_value. |
| edg/core/CompiledDesignExport.py | Adds CompiledParam.error field and maps ErrorValue into export output. |
| edg/core/Binding.py | Extends UnarySetOpBinding to populate empty_value into expr protos. |
| edg/core/ArrayExpr.py | Moves reductions into typed array subclasses and supplies explicit empty fallbacks. |
| edg/core/Array.py | Sets empty_value for FLATTEN to an explicitly empty array literal. |
| compiler/src/test/scala/edg/compiler/TunnelExportTest.scala | Updates test IR to pass emptyValue to UnarySetOp. |
| compiler/src/test/scala/edg/compiler/ExprEvaluateTest.scala | Updates unary-set evaluation tests to include emptyValue. |
| compiler/src/test/scala/edg/compiler/ExprEvaluatePartialTest.scala | Updates partial-eval tests for new UnarySetOp signature. |
| compiler/src/test/scala/edg/compiler/ConstPropAssignTest.scala | Adds tests for overassign errors producing ErrorValue and not over-propagating. |
| compiler/src/test/scala/edg/compiler/ConstPropArrayTest.scala | Updates array reduction tests with explicit emptyValue args. |
| compiler/src/test/scala/edg/compiler/CompilerEvaluationTest.scala | Updates compiler evaluation tests with explicit reduction empty fallbacks. |
| compiler/src/main/scala/edg/util/DependencyGraph.scala | Adds stop option to setValue to halt propagation without crashing. |
| compiler/src/main/scala/edg/ExprBuilder.scala | Updates UnarySetOp builder signature and adds literal Error helper. |
| compiler/src/main/scala/edg/compiler/ValueExprMap.scala | Updates visitor signature for unary-set mapping to include emptyValue. |
| compiler/src/main/scala/edg/compiler/ExprValue.scala | Makes ErrorValue IR-representable; supports literal conversion + optional messages. |
| compiler/src/main/scala/edg/compiler/ExprToString.scala | Adds rendering for error literals; updates unary-set map signature. |
| compiler/src/main/scala/edg/compiler/ExprEvaluatePartial.scala | Updates partial evaluation to evaluate/track refs in both vals and emptyValue. |
| compiler/src/main/scala/edg/compiler/ExprEvaluate.scala | Uses explicit emptyValue for empty arrays; converts prior hard errors to ErrorValue(Some(...)). |
| compiler/src/main/scala/edg/compiler/DesignRefsValidate.scala | Collects references from both unary-set vals and emptyValue. |
| compiler/src/main/scala/edg/compiler/ConstProp.scala | Reworks error handling to store ErrorValue in graph and track overassigns separately. |
| compiler/src/main/scala/edg/compiler/Compiler.scala | Bumps expected proto version; updates FLATTEN nodes to provide an empty array fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+77
to
82
| class ErrorValue(NamedTuple): | ||
| msg: str | ||
|
|
||
|
|
||
| LitLeafTypes = Union[bool, float, "Range", str, ErrorValue] # TODO for Range: fix me, this prevents a circular import | ||
| LitTypes = Union[LitLeafTypes, List[LitLeafTypes]] |
| @@ -1,5 +1,6 @@ | |||
| from __future__ import annotations | |||
|
|
|||
| from abc import abstractmethod | |||
| "calcHull" -> ValueExpr.Assign( | ||
| Ref("hull"), | ||
| ValueExpr.UnarySetOp(Op.HULL, ValueExpr.MapExtract(Ref("ports"), Ref("floatVal"))) | ||
| ValueExpr.UnarySetOp(Op.HULL, ValueExpr.MapExtract(Ref("ports"), Ref("floatVal")), ValueExpr.Literal(0.0)) |
|
|
||
| evalTest.map( | ||
| ValueExpr.UnarySetOp(expr.UnarySetExpr.Op.SUM, ValueExpr.MapExtract(Ref("container"), "inner")) | ||
| ValueExpr.UnarySetOp(expr.UnarySetExpr.Op.SUM, ValueExpr.MapExtract(Ref("container"), "inner"), ValueExpr.Literal(0, 0)) |
| throw new NotImplementedError(s"Undefined mapUnary for $unary") | ||
| def mapUnarySet(unarySet: expr.UnarySetExpr, vals: OutputType): OutputType = | ||
| def mapUnarySet(unarySet: expr.UnarySetExpr, vals: OutputType, emptyValue: OutputType): OutputType = | ||
| throw new NotImplementedError(s"Undefined mapBinarySet for $unarySet") |
Comment on lines
78
to
+84
| addArray(constProp, Seq(), 4, { i => ValueExpr.Literal(i.toFloat, (i + 4).toFloat) }, Seq("ports"), Seq("param")) | ||
|
|
||
| constProp.addDeclaration(DesignPath() + "reduce", ValInit.Integer) | ||
| constProp.addAssignExpr( | ||
| IndirectDesignPath() + "reduce", | ||
| ValueExpr.UnarySetOp(Op.INTERSECTION, ValueExpr.MapExtract(Ref("ports"), "param")) | ||
| ValueExpr.UnarySetOp( | ||
| Op.INTERSECTION, |
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.
Changes the IR and compiler to define the empty array output of array reduction operations in the frontend, allowing better type propagation (especially disambiguating float 0 and range 0 for sums).
Changes the error handling to generate an error value instead of crashing the compiler, allowing a partial compile instead of a unhelpful stack trace.
In the compiler, moves the param errors into the param value, while tracking overassigns separately because of the immutability of the param value system.
Also tightens up what reduction operations are allowed in the array subtypes.
Resolves #223