Skip to content

feat: added ExpressionRewriter class and SympyExpressionRewriter subclass#205

Merged
Brendan-Reid1991 merged 107 commits intomainfrom
symbolic_manipulator
Jun 23, 2025
Merged

feat: added ExpressionRewriter class and SympyExpressionRewriter subclass#205
Brendan-Reid1991 merged 107 commits intomainfrom
symbolic_manipulator

Conversation

@Brendan-Reid1991
Copy link
Copy Markdown
Contributor

Description

A proper implementation of the Gnarly Symbolic Expression class GSE from gse_prototyping branch.

What this PR does:

  • Adds capability to interact with symbolic expressions via ExpressionRewriter. Features implemented in the base class include:
    • Returning all variables in the expression,
    • Returning the expression as a tuple of its individual terms,
    • expanding all the brackets in the expression, and
    • focusing on particular variables, such that terms that do not include those variables are obfuscated.
  • Adds the capability to act on entire routines with this object, via ResourceRewriter.
    • At present this class doesn't do anything other than permit access to ExpressionRewriter on the top level resource, in a future PR this will be amended such that a sequence of instructions can be applied to a routine hierarchy.
  • SympyExpressionRewriter is the genuine inheritor of GSE. Subclasses from ExpressionRewriter and adds some functionality.
    • list_argument_of_function and all_functions_and_arguments are not abstract methods of ExpressionRewriter, but I am happy for these to be made abstract methods.

What this PR does not do

  • Any kind of assumptions, substitutions, or actual gnarly manipulation.
  • This is primarily intended to settle on the interface first and foremost, and then feature development can begin apace.
  • This PR also does not have the methods return the repr of the underlying expression, because that ended up being more difficult than expected to get it to appear nicely in interactive environments. Will be left for a future PR.

Tests

  • I have created basic_rewriter_tests.py that contain tests that all rewriters (regardless of backend) should pass.
    • It has a few fixtures defined on strings, parsed by the backend.as_expression method, and these can be used by the subclasses.
  • I went back and forth multiple times about creating a base class for tests, given than sympy is our only backend. But, in the event we do introduce another symbolic backend this will prevent a major refactor, or test rewriting.

Please pay attention to the typing and ensure I have it correct, I spent far too long on it and I'm still not 100% confident with it.

Please verify that you have completed the following steps

  • I have self-reviewed my code.
  • I have included test cases validating introduced feature/fix.
  • I have updated documentation.

Brendan-Reid1991 and others added 10 commits June 12, 2025 16:03
* chore: update poetry.lock file for dependabot

* chore: update lock file

* chore: remove h11 as transient dependency

* chore: update poetry lock
* refactor: made pandas and plotly optional installs

* Added checks to ensure tests do not fail unexpectedly

* ran isort

* Added .venv to .gitignore

* Addition of pandas stubs changed behaviour of scipy stubs; quick fix

* renamed visualization->interactive

* updated extras flag

* renamed vis->viz

* chore: rename nlz to ntz (#225)

* chore: rename nlz to ntz

* chore: add deprecated class alias.

* fix: test and linter

* added plotly-stubs; may revert

* Removed type: ignore

* removed type: ignore; updated error message

* fixing plotly-stubs install

* Removed plotly-stubs entirely.

* added pytest.importorskip to module

* Added noqa to pass flake8

* Ran isort

* isort versioning mismatch

* Streamlined some aspects of the TreeMap class; removed unnecessary checks

* Removing unused imports

* fix: Fix typing in analysis module

---------

Co-authored-by: pqvr <work.vrpq@gmail.com>
Co-authored-by: Konrad Jałowiecki <dexter2206@gmail.com>
@Brendan-Reid1991
Copy link
Copy Markdown
Contributor Author

@dexter2206 changing SymbolicBackend.free_symbols to return an iterable of T created a crazy can of worms, and meant that I essentially had to cast it to str in every place it was called, and ultimately it felt like it was outside of the scope of this PR because of the number of changes I had to make, and casting something to str repeatedly doesn't feel very nice.

I explored the idea of renaming free_symbols to free_symbol_names to keep functionality, and repurposing free_symbols to return T, but again that felt like a larger change than appropriate to tack onto this PR. I think it's something we should explore though.

I then opted for a decidedly worse solution, but one that requires minimal changes to the API: the substitute function in sympy_backend now correctly finds the symbols in the expression for substitution. I would be open to adding in a function to sympy_backend like :

def get_symbol(self, symbol_name: str)-> Expr

which just moves that functionality from SympyExpressionRewriter into the backend, but it's only use is in substitute at this stage.

I expect another round or two of changes, so take a look and let me know!

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 16, 2025

badge

Code Coverage Summary

Filename                                                 Stmts    Miss  Cover    Missing
-----------------------------------------------------  -------  ------  -------  --------------------------------------
src/bartiq/__init__.py                                       4       0  100.00%
src/bartiq/_routine.py                                     167       4  97.60%   124, 186, 258, 371
src/bartiq/errors.py                                         3       0  100.00%
src/bartiq/repetitions.py                                  150       4  97.33%   213, 272, 313-317
src/bartiq/transform.py                                     82       4  95.12%   37, 42, 213, 283
src/bartiq/verification.py                                  29       0  100.00%
src/bartiq/visualizations.py                                67      67  0.00%    16-218
src/bartiq/analysis/__init__.py                              0       0  100.00%
src/bartiq/analysis/optimization.py                        161     161  0.00%    14-373
src/bartiq/analysis/_rewriters/__init__.py                   0       0  100.00%
src/bartiq/analysis/_rewriters/expression_rewriter.py       46       8  82.61%   85, 111-117, 120
src/bartiq/analysis/_rewriters/sympy_rewriter.py            38       3  92.11%   54, 61, 118
src/bartiq/compilation/__init__.py                           3       0  100.00%
src/bartiq/compilation/_common.py                           41       0  100.00%
src/bartiq/compilation/_compile.py                         177       6  96.61%   95, 101, 164, 166, 168, 245
src/bartiq/compilation/_evaluate.py                         32       0  100.00%
src/bartiq/compilation/derived_resources.py                 22       0  100.00%
src/bartiq/compilation/postprocessing.py                    10       0  100.00%
src/bartiq/compilation/preprocessing.py                     76      13  82.89%   44-79
src/bartiq/integrations/__init__.py                          7       1  85.71%   22
src/bartiq/integrations/latex.py                           116       9  92.24%   56, 132, 178-184, 216
src/bartiq/integrations/jupyter/__init__.py                  0       0  100.00%
src/bartiq/integrations/jupyter/routine_explorer.py         52      51  1.92%    16-92
src/bartiq/symbolics/__init__.py                             2       0  100.00%
src/bartiq/symbolics/ast_parser.py                         108       5  95.37%   216, 286, 305, 307-308
src/bartiq/symbolics/backend.py                             63      10  84.13%   28, 32, 36, 40, 44, 48, 52, 56, 60, 64
src/bartiq/symbolics/interpreter.py                         21       0  100.00%
src/bartiq/symbolics/sympy_backend.py                      152       6  96.05%   115, 133, 180, 263-264, 323
src/bartiq/symbolics/sympy_interpreter.py                   94      15  84.04%   110-111, 119-138, 175-180
src/bartiq/symbolics/sympy_serializer.py                    25       0  100.00%
TOTAL                                                     1748     367  79.00%

Results for commit: 347d853

Minimum allowed coverage is 0%

♻️ This comment has been updated with latest results

@Brendan-Reid1991 Brendan-Reid1991 dismissed mstechly’s stale review June 23, 2025 15:35

Dismissing review to finalise this PR. Michal communicated that he was satisfied with the state of the PR after our discussions with Konrad.

@Brendan-Reid1991 Brendan-Reid1991 merged commit 74ed648 into main Jun 23, 2025
9 of 10 checks passed
@Brendan-Reid1991 Brendan-Reid1991 deleted the symbolic_manipulator branch June 23, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

6 participants