Skip to content

[Rewrite] sqrt(x**2) -> abs(x) (#59)#63

Closed
ArthurPendragn wants to merge 4 commits into
deem-data:mainfrom
ArthurPendragn:main
Closed

[Rewrite] sqrt(x**2) -> abs(x) (#59)#63
ArthurPendragn wants to merge 4 commits into
deem-data:mainfrom
ArthurPendragn:main

Conversation

@ArthurPendragn
Copy link
Copy Markdown
Contributor

Closes #59

  • New ops: SQRT, ABS, SQUARE added to NumericOpType + dispatch
  • Constructor refactor: NumericOp now accepts either a func or a type directly, enabling synthetic nodes (e.g. the replacement ABS op)
  • x**2 support: BinOp(**2) now canonicalizes to SQUARE, making it rewrite-eligible
  • New rewrite pass: eliminate_sqrt_square replaces SQUARE → SQRT chains with ABS
  • Config toggle: AlgebraicRewritesConfig.sqrt_square (default on)
  • 11 new tests

 - New ops: SQRT, ABS, SQUARE added to NumericOpType + dispatch
  - Constructor refactor: NumericOp now accepts either a func or a type directly, enabling synthetic nodes (e.g. the replacement ABS op)
  - x**2 support: BinOp(**2) now canonicalizes to SQUARE, making it rewrite-eligible
  - New rewrite pass: eliminate_sqrt_square replaces SQUARE → SQRT chains with ABS
  - Config toggle: AlgebraicRewritesConfig.sqrt_square (default on)
  - 11 new tests
Copy link
Copy Markdown
Collaborator

@e-strauss e-strauss left a comment

Choose a reason for hiding this comment

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

Hi @ArthurPendragn thanks for your first contribution :)

Overall the PR looks good I think, I added some minor comments and please take a look at the failing tests. you can also run the test locally with:

cd stratum
pytest -v tests

You can just add commits to this branch, I will squash them during merge.

Comment thread stratum/optimizer/_numeric_rewrites.py Outdated
Comment thread stratum/optimizer/_numeric_rewrites.py Outdated
Comment thread stratum/tests/logical_optimizer/algebraic_rewrites/test_numeric.py
Comment thread stratum/optimizer/ir/_numeric_ops.py
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

❌ Patch coverage is 93.65079% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
stratum/optimizer/ir/_numeric_ops.py 90.47% 2 Missing and 2 partials ⚠️
Flag Coverage Δ
unittests 91.59% <93.65%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
stratum/optimizer/_algebraic_rewrites.py 100.00% <100.00%> (ø)
stratum/optimizer/_numeric_rewrites.py 100.00% <100.00%> (ø)
stratum/optimizer/ir/_numeric_ops.py 94.93% <90.47%> (-5.07%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@e-strauss e-strauss left a comment

Choose a reason for hiding this comment

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

LGTM

@e-strauss e-strauss closed this in e9e60cc May 7, 2026
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.

[Rewrite] sqrt(x**2) -> abs(x)

2 participants