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

Make assumptions about indexed symbols #13992

Closed
normalhuman opened this issue Jan 22, 2018 · 6 comments · Fixed by #16085
Closed

Make assumptions about indexed symbols #13992

normalhuman opened this issue Jan 22, 2018 · 6 comments · Fixed by #16085

Comments

@normalhuman
Copy link
Contributor

normalhuman commented Jan 22, 2018

I think this should be possible to do:

>>> x = IndexedBase("x", positive=True)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ubuntu/sympy/sympy/tensor/indexed.py", line 403, in __new__
    obj = Expr.__new__(cls, label, **kw_args)
TypeError: __new__() got an unexpected keyword argument 'positive'

Without assumptions, it's hard to do algebra with indexed symbols, e.g., get sqrt(x[1]**2) to simplify to x[1].

I understand that the class Indexed inherits from Expr, not from Symbol, and there are no assumptions on Expr. Still, we already have some quasi-assumptions on Indexed and IndexedBase in the form of is_commutative=True class property. And the objects of class Idx, which also inherits from Expr, have them in the form obj._assumptions["finite"] = True as seen here.

Could IndexedBase.__new__ parse and use the **kwargs that it accepts, instead of trying to pass them to Expr.__new__?

Optionally, maybe even Indexed.__new__ could do that, so an assumption can be added to a particular indexed object?

By the way, any attempt to pass **kwargs to Expr.__new__ is a potential bug because this method, inherited from Basic.__new__, does not accept any keyword arguments.

@Nirvan101
Copy link

Can I work on this?

@normalhuman
Copy link
Contributor Author

Yes, you can.

@jlumpe
Copy link
Contributor

jlumpe commented Oct 17, 2018

@Nirvan101 any progress on this?

@PymZoR
Copy link

PymZoR commented Nov 1, 2018

+1

@asmeurer
Copy link
Member

asmeurer commented Nov 1, 2018

Since it doesn't subclass from Symbol, it should reapply the logic in the Symbol constructor:

sympy/sympy/core/symbol.py

Lines 233 to 235 in cd98ba0

is_commutative = fuzzy_bool(assumptions.get('commutative', True))
assumptions['commutative'] = is_commutative
obj._assumptions = StdFactKB(assumptions)

Specifically, create a StdFactKB from the assumptions that are passed in and set obj._assumptions to it.

bsamseth added a commit to bsamseth/sympy that referenced this issue Feb 26, 2019
``Indexed`` and ``IndexedBase`` now accepts a new keyword argument
allowing the specification of assumptions on indexed symbols.
@bsamseth
Copy link
Contributor

As this had been open for a while, I've made a PR that I believe resolves this issue (#16085). I based the changes on @asmeurer's comment, and have reused the logic applied in Symbol.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants