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 invert_real more robust #9628

Merged
merged 5 commits into from Jul 17, 2015

Conversation

Projects
None yet
3 participants
@aktech
Copy link
Member

aktech commented Jul 7, 2015

Improvements over Abs.
Returns domain of invert for Abs functions.

TODO

  • Abs
  • Document new changes
  • Add More tests

# minus_n = Intersection(Interval(-oo, 0), FiniteSet(-n))
# plus_n = Intersection(Interval(0, oo), FiniteSet(n))
# assert solveset(abs(x) - n, x) == Union(minus_n, plus_n)

This comment has been minimized.

@aktech

aktech Jul 7, 2015

Author Member

Will pass after #9627 gets Merged.

@aktech aktech force-pushed the aktech:invert-real branch from 1967c6a to 839000c Jul 7, 2015

@aktech aktech self-assigned this Jul 7, 2015

@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 8, 2015

@gxyd

This comment has been minimized.

Copy link
Member

gxyd commented Jul 8, 2015

I have two things to say:

  1. Why you the import inside a loop? Importing it again and again.
  2. With your current code a TypeError is raised for invert_real(exp(Abs(x**2 - 1)), y, x) and I am not sure for reason of happening.
@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 8, 2015

@gxyd Yeah that's wrong. I will fix it, write now I am just trying to build a rough logic of how things will work, refactoring will go at last, when everything is done.

@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 8, 2015

  1. With your current code a TypeError is raised for invert_real(exp(Abs(x**2 - 1)), y, x) and I am not sure for reason of happening

That's a problem with the logic. Thanks, I will fix it.

@aktech aktech force-pushed the aktech:invert-real branch from 1f491db to 4e9fb02 Jul 8, 2015

@@ -40,11 +40,11 @@ def invert_real(f_x, y, x):
>>> from sympy import tan, Abs, exp
>>> from sympy.abc import x, y, n
>>> invert_real(exp(Abs(x)), y, x)
(x, {-log(y), log(y)})
(x, Intersection([0, oo), {log(y)}) U Intersection((-oo, 0], {-log(y)}))

This comment has been minimized.

@hargup

hargup Jul 9, 2015

Member

You should explain the idea behind this PR in the docstring, then maybe you should put these examples in a different "section".

This comment has been minimized.

@aktech

aktech Jul 9, 2015

Author Member

Yes, sure.

@hargup

This comment has been minimized.

Copy link
Member

hargup commented Jul 9, 2015

I have two things to say:

  1. Why you the import inside a loop? Importing it again and again.

@aktech did you use force push? We were talking about that at the SciPy and as a general practice we want to discourage they make it hard to see the changes that occur over time. If you think neat history is important you can rebase the PR after it is ready.

@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 9, 2015

@aktech did you use force push?

Yes, I did. I will keep this in mind from next time.

@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 9, 2015

@hargup
BTW, what's your general view on the current approach.

_a.append(FiniteSet(g_y).intersect(Interval(0, oo)))

g_ys = Union(*[element for element in _a if element.args])

return _invert_real(f.args[0],
Union(g_ys, imageset(Lambda(n, -n), g_ys)), symbol)

This comment has been minimized.

@hargup

hargup Jul 9, 2015

Member

You current approach is not quite right, see:

In [12]: invert_real(Abs(x), -y, x)
Out[12]: (x, Intersection([0, oo), {y}) U Intersection((-oo, 0], {-y})) # should be (x, Intersection([0, oo), {-y}) U Intersection((-oo, 0], {y})) 
In [14]: invert_real(Abs(x), -y, x)[1].subs('y', 1)
Out[14]: {-1, 1} # should be EmptySet

Rather what you need to do is replace the last return call with something like, after that line 79 will also become redundant.

         return _invert_real(f.args[0],
                             Union(imageset(Lambda(n, n), g_ys).intersect(Interval(0, oo)),
                                 imageset(Lambda(n, -n), g_ys).intersect(Interval(-oo, 0))),
                             symbol)

This comment has been minimized.

@aktech

aktech Jul 10, 2015

Author Member

Ah! that's clean! Thanks!

aktech added some commits Jul 6, 2015

Improve Abs Invert return call
Signed-off-by: AMiT Kumar <dtu.amit@gmail.com>
UnXFAIL few test cases
Signed-off-by: AMiT Kumar <dtu.amit@gmail.com>

@aktech aktech force-pushed the aktech:invert-real branch from 4237cc8 to c4a28a1 Jul 10, 2015

@hargup

This comment has been minimized.

Copy link
Member

hargup commented Jul 10, 2015

I tried this PR with a little more complicated example and it broke:

solveset_real(Abs(Abs(x+3) - a)-b, x)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/home/hargup/210fs/sympy/sympy/sets/fancysets.py in _contains(self, other)
    238             try:
--> 239                 if soln in self.base_set:
    240                     return S.true

/home/hargup/210fs/sympy/sympy/sets/sets.py in __contains__(self, other)
    516         if symb not in (true, false):
--> 517             raise TypeError('contains did not evaluate to a bool: %r' % symb)
    518         return bool(symb)

TypeError: contains did not evaluate to a bool: And(b < oo, b >= 0)

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
<ipython-input-31-09b7db5a7787> in <module>()
----> 1 solveset_real(Abs(Abs(x+3) - a)-b, x)

/home/hargup/210fs/sympy/sympy/solvers/solveset.py in solveset_real(f, symbol)
    428             result = result + solns
    429     else:
--> 430         lhs, rhs_s = invert_real(f, 0, symbol)
    431         if lhs == symbol:
    432             result = rhs_s

/home/hargup/210fs/sympy/sympy/solvers/solveset.py in invert_real(f_x, y, x)
     55     y = sympify(y)
     56     if not y.has(x):
---> 57         return _invert_real(f_x, FiniteSet(y), x)
     58     else:
     59         raise ValueError(" y should be independent of x ")

/home/hargup/210fs/sympy/sympy/solvers/solveset.py in _invert_real(f, g_ys, symbol)
     87         g, h = f.as_independent(symbol)
     88         if g != S.Zero:
---> 89             return _invert_real(h, imageset(Lambda(n, n - g), g_ys), symbol)
     90 
     91     if f.is_Mul:

/home/hargup/210fs/sympy/sympy/solvers/solveset.py in _invert_real(f, g_ys, symbol)
     81                                 imageset(Lambda(n, n), g_ys).intersect(Interval(0, oo)),
     82                                 imageset(Lambda(n, -n), g_ys).intersect(Interval(-oo, 0))),
---> 83                             symbol)
     84 
     85     if f.is_Add:

/home/hargup/210fs/sympy/sympy/solvers/solveset.py in _invert_real(f, g_ys, symbol)
     87         g, h = f.as_independent(symbol)
     88         if g != S.Zero:
---> 89             return _invert_real(h, imageset(Lambda(n, n - g), g_ys), symbol)
     90 
     91     if f.is_Mul:

/home/hargup/210fs/sympy/sympy/sets/sets.py in imageset(*args)
   1950     set = args[-1]
   1951 
-> 1952     r = set._eval_imageset(f)
   1953     if isinstance(r, ImageSet):
   1954         f, set = r.args

/home/hargup/210fs/sympy/sympy/sets/sets.py in _eval_imageset(self, f)
   1282 
   1283     def _eval_imageset(self, f):
-> 1284         return Union(imageset(f, arg) for arg in self.args)
   1285 
   1286     def as_relational(self, symbol):

/home/hargup/210fs/sympy/sympy/sets/sets.py in __new__(cls, *args, **kwargs)
   1145                 return sum(map(flatten, arg), [])
   1146             raise TypeError("Input must be Sets or iterables of Sets")
-> 1147         args = flatten(args)
   1148 
   1149         # Union of no sets is EmptySet

/home/hargup/210fs/sympy/sympy/sets/sets.py in flatten(arg)
   1143                     return [arg]
   1144             if iterable(arg):  # and not isinstance(arg, Set) (implicit)
-> 1145                 return sum(map(flatten, arg), [])
   1146             raise TypeError("Input must be Sets or iterables of Sets")
   1147         args = flatten(args)

/home/hargup/210fs/sympy/sympy/sets/sets.py in flatten(arg)
   1143                     return [arg]
   1144             if iterable(arg):  # and not isinstance(arg, Set) (implicit)
-> 1145                 return sum(map(flatten, arg), [])
   1146             raise TypeError("Input must be Sets or iterables of Sets")
   1147         args = flatten(args)

/home/hargup/210fs/sympy/sympy/sets/sets.py in <genexpr>(.0)
   1282 
   1283     def _eval_imageset(self, f):
-> 1284         return Union(imageset(f, arg) for arg in self.args)
   1285 
   1286     def as_relational(self, symbol):

/home/hargup/210fs/sympy/sympy/sets/sets.py in imageset(*args)
   1950     set = args[-1]
   1951 
-> 1952     r = set._eval_imageset(f)
   1953     if isinstance(r, ImageSet):
   1954         f, set = r.args

/home/hargup/210fs/sympy/sympy/sets/sets.py in _eval_imageset(self, f)
   1398 
   1399     def _eval_imageset(self, f):
-> 1400         return Intersection(imageset(f, arg) for arg in self.args)
   1401 
   1402     def _contains(self, other):

/home/hargup/210fs/sympy/sympy/sets/sets.py in __new__(cls, *args, **kwargs)
   1381         # Reduce sets using known rules
   1382         if evaluate:
-> 1383             return Intersection.reduce(args)
   1384 
   1385         return Basic.__new__(cls, *args)

/home/hargup/210fs/sympy/sympy/sets/sets.py in reduce(args)
   1435             if s.is_FiniteSet:
   1436                 other_args = [a for a in args if a != s]
-> 1437                 res = FiniteSet(*[x for x in s
   1438                              if all(other.contains(x) == True for other in other_args)])
   1439                 unk = [x for x in s

/home/hargup/210fs/sympy/sympy/sets/sets.py in <listcomp>(.0)
   1436                 other_args = [a for a in args if a != s]
   1437                 res = FiniteSet(*[x for x in s
-> 1438                              if all(other.contains(x) == True for other in other_args)])
   1439                 unk = [x for x in s
   1440                        if any(other.contains(x) not in (True, False) for other in other_args)]

/home/hargup/210fs/sympy/sympy/sets/sets.py in <genexpr>(.0)
   1436                 other_args = [a for a in args if a != s]
   1437                 res = FiniteSet(*[x for x in s
-> 1438                              if all(other.contains(x) == True for other in other_args)])
   1439                 unk = [x for x in s
   1440                        if any(other.contains(x) not in (True, False) for other in other_args)]

/home/hargup/210fs/sympy/sympy/sets/sets.py in contains(self, other)
    279         """
    280         other = sympify(other, strict=True)
--> 281         ret = self._contains(other)
    282         if ret is None:
    283             if all(Eq(i, other) == False for i in self):

/home/hargup/210fs/sympy/sympy/sets/fancysets.py in _contains(self, other)
    240                     return S.true
    241             except TypeError:
--> 242                 if soln.evalf() in self.base_set:
    243                     return S.true
    244         return S.false

/home/hargup/210fs/sympy/sympy/sets/sets.py in __contains__(self, other)
    515         symb = self.contains(other)
    516         if symb not in (true, false):
--> 517             raise TypeError('contains did not evaluate to a bool: %r' % symb)
    518         return bool(symb)
    519 

TypeError: contains did not evaluate to a bool: And(b < oo, b >= 0)
@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 11, 2015

Yeah, also the @gxyd 's example: invert_real(exp(Abs(x**2 - 1)), y, x) gave me the same TypeError.

The problem is basically with this: x in Interval(1, oo) (x in some_foo_Interval) :

In [6]: x = Symbol('x')

In [7]: x in Interval(1, oo)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-7-2b2509d21b06> in <module>()
----> 1 x in Interval(1, oo)

/home/amit/Desktop/sympy/sympy/sets/sets.pyc in __contains__(self, other)
    515         symb = self.contains(other)
    516         if symb not in (true, false):
--> 517             raise TypeError('contains did not evaluate to a bool: %r' % symb)
    518         return bool(symb)
    519 

TypeError: contains did not evaluate to a bool: And(x < oo, x >= 1)
@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 11, 2015

@hargup

Symbol('x', real=True) in S.Reals returns True then shouldn't
Symbol('x') in S.Reals should return None rather than a TypeError.

In [8]: Symbol('x', real=True) in S.Reals
Out[8]: True
In [12]: Symbol('x') in S.Reals
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-12-07b5591a0a1c> in <module>()
----> 1 Symbol('x') in S.Reals

/home/amit/Desktop/myrepo/sympy/sympy/sets/sets.pyc in __contains__(self, other)
    515         symb = self.contains(other)
    516         if symb not in (true, false):
--> 517             raise TypeError('contains did not evaluate to a bool: %r' % symb)
    518         return bool(symb)
    519 

TypeError: contains did not evaluate to a bool: And(x < oo, x > -oo)

I reported here: #9644 (Close it, if it's invalid).

@hargup

This comment has been minimized.

Copy link
Member

hargup commented Jul 11, 2015

in can only return True or False, you replace the internal calls of in with contains. For example use Interval(0, oo).contains(x) instead of x in Interval(0, oo)

@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 11, 2015

Yeah, thanks, Interval(0, oo).contains(x) should work.
Then, #9644 is invalid now.

Replace 'in' with contains in ImageSet
Signed-off-by: AMiT Kumar <dtu.amit@gmail.com>
@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 11, 2015

After 4th commit:

In [6]: invert_real(Abs(Abs(x+3) - a)-b, 0, x)
Out[6]: 
(x, ([-3, ∞) ∩ {a + b - 3} ∩ {-n + a - 3 | n ∊ (-∞, 0]}) ∪ ((-∞, -3] ∩ {-a + b - 3} ∩ {n - a - 3 | n ∊ [0, ∞)}) ∪ ([-3, ∞) ∩ {a
 - b - 3} ∩ {-n + a - 3 | n ∊ [0, ∞)}) ∪ ((-∞, -3] ∩ {-a - b - 3} ∩ {n - a - 3 | n ∊ (-∞, 0]}))

It seems like if it returns a lot of unevaluated ImageSets() objects.

@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 12, 2015

The output is correct as well:

In [11]: ans = solveset_real(Abs(Abs(x+3) - a) - b, x)

In [12]: ans.subs({a : 0, b : 1})
Out[12]: {-4, -2}
In [15]: f = Abs(Abs(x+3) - a) - b

In [22]: f
Out[22]: -b + │a - │x + 3││

In [19]: e = f.subs({a : 0, b : 1})

In [21]: e
Out[21]: │x + 3- 1

In [20]: solveset_real(e, x)
Out[20]: {-4, -2}

So, returning unevaluated in ImageSet works.

@aktech

This comment has been minimized.

Copy link
Member Author

aktech commented Jul 12, 2015

Now For Pow :

The invert_real(2**x, y, x) returns (x, {log(y)/log(2)}) , so we need to output somehow stating y belongs to (0, oo) , but no set operation on the FiniteSet {log(y)/log(2)} can state that y belongs to (0, oo) except Intersection((-oo, oo), {log(y)/log(2)}) which is trivial and solveset_real already returns that. Is there anything else we can do without changing the API?

In [6]: invert_real(2**x, y, x)
Out[6]: (x, {log(y)/log(2)})

In [7]: solveset_real(2**x - y, x)
Out[7]: Intersection((-oo, oo), {log(y)/log(2)})
@hargup

This comment has been minimized.

Copy link
Member

hargup commented Jul 13, 2015

Intersection((-oo, oo), {log(y)/log(2)}) doesn't seem right. I'll think about it again after getting back home. I guess you can skip such intersection in Pow as log(y) implicitly has the domain (0, oo).

Document changes and add more tests
Signed-off-by: AMiT Kumar <dtu.amit@gmail.com>
@hargup

This comment has been minimized.

Copy link
Member

hargup commented Jul 17, 2015

+1

@aktech aktech changed the title [WIP] Make invert_real more robust Make invert_real more robust Jul 17, 2015

hargup added a commit that referenced this pull request Jul 17, 2015

Merge pull request #9628 from aktech/invert-real
Make invert_real more robust

@hargup hargup merged commit 2ae9d7b into sympy:master Jul 17, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@aktech aktech deleted the aktech:invert-real branch Jul 25, 2015

@aktech aktech referenced this pull request Aug 10, 2015

Merged

Remove autosimp from exp #9766

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