Skip to content

Fix operator precedence bug in dag_from_flow and add prep-before-meas constraint#168

Merged
masa10-f merged 4 commits intomasterfrom
fix/dag-self-loop-and-prep-before-meas-constraint
Mar 9, 2026
Merged

Fix operator precedence bug in dag_from_flow and add prep-before-meas constraint#168
masa10-f merged 4 commits intomasterfrom
fix/dag-self-loop-and-prep-before-meas-constraint

Conversation

@masa10-f
Copy link
Collaborator

@masa10-f masa10-f commented Mar 9, 2026

Summary

  • Bug fix: feedforward.dag_from_flow had an operator precedence bug where self-loops were only removed from zflow but not xflow. The expression xflow.get(node, set()) | zflow.get(node, set()) - {node} evaluates as xflow | (zflow - {node}) because - binds tighter than |. Adding parentheses fixes this: (xflow | zflow) - {node}.
  • New constraint: schedule_solver._add_constraints now enforces that every non-input, non-output node must be prepared strictly before it is measured (node2prep[node] < node2meas[node]).
  • Test: Added integration test test_minimize_space_cpsat_prepares_node_before_measuring_it to verify this constraint under the MINIMIZE_SPACE CP-SAT strategy. Also fixed RUF031 lint errors (redundant parentheses in tuple subscripts).

Test plan

  • pytest tests/test_scheduler_integration.py — all 21 tests pass
  • pytest — all 315 tests pass
  • ruff check — no errors
  • mypy — no errors
  • pyright — no errors

🤖 Generated with Claude Code

… constraint

- Fix operator precedence bug in feedforward.dag_from_flow: self-loops were only
  removed from zflow but not xflow due to | having lower precedence than -.
  Wrap the union in parentheses so self-loops are correctly excluded from both.
- Add constraint to schedule_solver that every non-input, non-output node must
  be prepared strictly before it is measured.
- Add integration test verifying MINIMIZE_SPACE enforces preparation before
  measurement, and fix RUF031 lint errors (remove redundant parentheses in
  tuple subscripts).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.15%. Comparing base (4b2655f) to head (f07fbd8).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #168      +/-   ##
==========================================
- Coverage   84.17%   84.15%   -0.03%     
==========================================
  Files          22       22              
  Lines        2603     2606       +3     
  Branches      474      476       +2     
==========================================
+ Hits         2191     2193       +2     
  Misses        308      308              
- Partials      104      105       +1     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- Update CHANGELOG with fixed bug, new constraint, and new test
- Narrow types via local variables before comparisons to fix
  mypy/pyright operator errors on int | None dict values

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@masa10-f masa10-f self-assigned this Mar 9, 2026
@masa10-f masa10-f added the bug Something isn't working label Mar 9, 2026
@masa10-f masa10-f requested a review from d1ssk March 9, 2026 05:17
Copy link
Collaborator

@d1ssk d1ssk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
It would be even better to add a test that verifies dag_from_flow() does not result in a self-loop when xflow[node] includes node.

@masa10-f masa10-f merged commit da38df7 into master Mar 9, 2026
23 checks passed
@masa10-f masa10-f deleted the fix/dag-self-loop-and-prep-before-meas-constraint branch March 9, 2026 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants