Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Substitution fails if an expression evaluates to a number and an attempt to define a function is made #143

Closed
dexter2206 opened this issue Nov 26, 2024 · 1 comment
Assignees
Labels
bug Something isn't working

Comments

@dexter2206
Copy link
Contributor

dexter2206 commented Nov 26, 2024

Describe the bug

When substituting expressions and defining function at the same time, the SympyBackend.substitute method fails with an AttributeError if sole substitution is enough to evaluate the expression to number.

How to Reproduce

The following code triggers an error:

backend = SympyBackend()

def f(x):
    return int(x) + 1 # The int here is crucial

def g(x):
    return x


expr = backend.as_expression("f(x)")

backend.substitute(expr, {"x": 10}, {"f": f, "g": g})

This is because after the substitution of x, the expression becomes constant, then this constant is converted to a native integer and the _define_function function fails for native integers (or floats). This, in turn, is caused by an incorrect usage of identity_for_numbers decorator which I didn't catch earlier.

Note that the problem is triggered only if the expression is not constant when entering substitute and becomes constant after defining one of the functions, before another attempt to define another function is made. So, for instance, the following code does not trigger an error:

backend.substitute(10, {"x": 10}, {"f": f, "g": g}) # We start with a constant
backend.substitute(expr, {"x": 10}, {"f": f}) # f evaluates to constant, but no function is is defined after f.

This is why we didn't spot the bug until now - its occurrence is rather rare.

Expected behavior

A constant resulting from variable substitution is returned.

Actual behavior

An AttributeError is raised:

AttributeError                            Traceback (most recent call last)
Cell In[10], line 1
----> 1 backend.substitute("x", {"x": 8}, {"f": f})

File ~/Projects/bartiq/src/bartiq/symbolics/sympy_backends.py:71, in identity_for_numbers.<locals>._inner(backend, expr, *args, **
kwargs)
     70 def _inner(backend: SympyBackend, expr: TExpr[S], *args: P.args, **kwargs: P.kwargs) -> T | Number:
---> 71     return expr if isinstance(expr, Number) else func(backend, expr, *args, **kwargs)

File ~/Projects/bartiq/src/bartiq/symbolics/sympy_backends.py:176, in SympyBackend.substitute(self, expr, replacements, functions_
map)
    167 @identity_for_numbers
    168 def substitute(
    169     self,
   (...)
    173     functions_map: Mapping[str, Callable[[TExpr[Expr]], TExpr[Expr]]] | None = None,
    174 ) -> TExpr[Expr]:
--> 176     symbols_in_expr = self.free_symbols_in(expr)
    177     restricted_replacements = [(symbols(old), new) for old, new in replacements.items() if old in symbols_in_expr]
    178     expr = expr.subs(restricted_replacements)

File ~/Projects/bartiq/src/bartiq/symbolics/sympy_backends.py:64, in empty_for_numbers.<locals>._inner(backend, expr, *args, **kwa
rgs)
     63 def _inner(backend: SympyBackend, expr: TExpr[S], *args: P.args, **kwargs: P.kwargs) -> Iterable[T]:
---> 64     return () if isinstance(expr, Number) else func(backend, expr, *args, **kwargs)

File ~/Projects/bartiq/src/bartiq/symbolics/sympy_backends.py:143, in SympyBackend.free_symbols_in(self, expr)
    140 @empty_for_numbers
    141 def free_symbols_in(self, expr: Expr) -> Iterable[str]:
    142     """Return an iterable over free symbol names in given expression."""
--> 143     return tuple(map(str, expr.free_symbols))

AttributeError: 'str' object has no attribute 'free_symbols'

Environment:**

This bug is environment-independent.

Additional context

N/A

@dexter2206 dexter2206 added the bug Something isn't working label Nov 26, 2024
@dexter2206 dexter2206 self-assigned this Nov 26, 2024
dexter2206 added a commit that referenced this issue Nov 26, 2024
mstechly pushed a commit that referenced this issue Nov 26, 2024
…144)

* Add test triggering #143

* Fix typo in test

* Fix root cause of the issue - invalid usage of identity_for_numbers

* Add a warning to the docstring to avoid incorrect usage of
identity_for_numbers

* Add comment about the function g
@dexter2206
Copy link
Contributor Author

Fixed via #144

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

1 participant