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

Point(1) in Interval(0, 2) == False ? - Fails in 1 dim #11683

Open
latot opened this issue Sep 30, 2016 · 23 comments
Open

Point(1) in Interval(0, 2) == False ? - Fails in 1 dim #11683

latot opened this issue Sep 30, 2016 · 23 comments
Labels

Comments

@latot
Copy link
Contributor

latot commented Sep 30, 2016

Hi, here a little weird thing

>>> Point(1) in Interval(0, 2)
False
>>> Point(1, 1) in Interval(0, 2)*Interval(0, 2)
True

as you can see Point works fine in 2 dims, but in 1 we get that.

Thx. Cya.

@ghost
Copy link

ghost commented Oct 2, 2016

It seems that the reason for this behaviour is that Point(1) is passed as a Point object from Set.contains() to Interval.contains(). This causes the following test to fail:

    def _contains(self, other):
        if not isinstance(other, Expr) or (
                other is S.Infinity or
                other is S.NegativeInfinity or
                other is S.NaN or
                other is S.ComplexInfinity) or other.is_real is False:
            return false

On the contrary, when ProductSet._contains() passes each dimension of Point(1,1) to set.contains() (and then to Interval._contains(), it extracts the Int objects from Point :

    def _contains(self, element):
        """
        """
        try:
            if len(element) != len(self.args):
                return false
        except TypeError:  # maybe element isn't an iterable
            return false
        return And(*
            [set.contains(item) for set, item in zip(self.sets, element)])

A possible fix is to add this line to Interval._contains():

    while isinstance(other, collections.Iterable) and len(other) == 1:
        other = other[0]

I can add this fix and send a pull request. I'm new to open-source contribution. I don't know if I should now wait for a confirmation by someone, or should I just proceed to applying the suggested fix

@smichr
Copy link
Member

smichr commented Oct 3, 2016

What you propose sounds fine to me.

@ghost
Copy link

ghost commented Oct 4, 2016

So I will work on this issue

@ghost
Copy link

ghost commented Oct 5, 2016

In commit 29268d3 a condition is added which requires the Point object to be at least 2-dimensional.

        if len(coords) < 2:
            raise ValueError(filldedent('''
                Point requires 2 or more coordinates or
                keyword `dim` > 1.'''))

Does it mean that we can close this issue?

@latot
Copy link
Contributor Author

latot commented Oct 5, 2016

mm, i think its not solved, in some way yes, but the origin of this not, of the first place, why dim 1 is disabled? al leas to me don't have to much sense...

@ghost
Copy link

ghost commented Oct 6, 2016

I tried to remove the condition that disallows one-dimensional points. However, several tests (which I think were added in commit b2aae57) failed. To me this indicates a firm intention to disallow one-dimensional points.

Since the condition and tests were added by @smichr , It would be good if he can comment on this. Are there strong reasons for excluding one-dimensional points?

@kshitij10496
Copy link
Member

@bhrzbbk Can you open a PR with your fix for this issue ?

@ghost
Copy link

ghost commented Nov 4, 2016

@kshitij10496 The failing tests are not just from geometry module. I tried to alter the tests and modify my fix to resolve this, but with no success. I have come to believe that my fix is violating (or has led to violation of) some fundamental assumptions. Given this status, do you still recommend opening a PR? Otherwise I can just leave the issue open.

@latot
Copy link
Contributor Author

latot commented Nov 4, 2016

mm, think would be nice can see the changes, maybe link your branch with the tmp fix here?, or open a [WIP] pr?

latot pushed a commit to latot/sympy that referenced this issue Nov 6, 2016
This is the example of the faulty behaviour from Issue sympy#11683:
>>> Point(1) in Interval(0, 2)
False
>>> Point(1, 1) in Interval(0, 2)*Interval(0, 2)
True

* Allow a `Point` object to have a dimension less than two again (this was disallowed in commit 29268d3)
* Explicitly extract the expression from the `Point` object when it is one-dimensional

Signed-off-by: latot <felipematas@yahoo.com>
@latot
Copy link
Contributor Author

latot commented Nov 6, 2016

Hi, i check your change, i think i know why this fails, basically you change two behaviors:

Point(1, dim=3) -> Point(1, 0, 0)
Someop(Point(0, 0), some-nopoint) -> some-nopoint transformed to point

This two cases now pass when it should return an error, the first case i think is a nice change, and it respect the idea written in the wiki i think we can open a new issue for that and can use it.
Now the second expression that need return an error, your change change all non points into points and execute the operation what you want to do, but it need send error because numbers don't are points, and the user should specify what point is, or if you like is possible open a new issue of transform numbers into points like 10 -> Point(10).

Thx. Cya.

@ghost
Copy link

ghost commented Nov 6, 2016

@latot Thanks for checking out the change.

About the first type of errors (upgrade to higher dimensionality), I also think that this is a good behaviour and probably the tests should be adapted to allow this.

As for the second type of errors, I guess silent upgrade of numbers to one-dimensional points can be dangerous. I'll try to address that in my fix; i.e. make sure that the errors of second type will not happen.

@latot
Copy link
Contributor Author

latot commented Nov 6, 2016

Hi!, only as a reference of the second behavior Point(1) + 10 there 10 is transformed to point.

Thx. Cya.

@oscarbenjamin
Copy link
Collaborator

This now gives an error:

In [184]: Point(1) in Interval(0, 2)                                                                                                                          
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-184-5a2ba01f9f49> in <module>
----> 1 Point(1) in Interval(0, 2)

~/current/sympy/sympy/sympy/geometry/point.py in __new__(cls, *args, **kwargs)
    127             raise TypeError(filldedent('''
    128                 Expecting sequence of coordinates, not `{}`'''
--> 129                                        .format(func_name(coords))))
    130         # A point where only `dim` is specified is initialized
    131         # to zeros.

TypeError: 
Expecting sequence of coordinates, not `int`

@sylee957
Copy link
Member

I personally don't think that the it's a good idea to make algebraic objects compatible with geometric elements without any context put into.

I think that for the original case, the relational should not be defined at all. But we may need some other representations like that can create a new region from line segments.

@latot
Copy link
Contributor Author

latot commented Sep 25, 2019

Is a point...

In my case i think there is already a relation, in a context of measure, they are types, a point and a interval have the same type.

I'm agree even if they are just different ways of measure is not enough to compare, there is the argument, "how are different types of measure, to compare would need to be the same type"

I think should be necessary say explicitily the type of comparation what we are doing.

To follow the ideas, maybe for example:

Limit the comparision to something just can be "in" other thing if they are the same type, 2 dim with 2 dim, 1 dim with 1 dim, and send an error of "wrong types to compare" when we send 2 different types.

here some cases:

Point(1) in Point(2) = False
Interval(1, 2) in interval(1, 3) = True
Interval(1, 2) in interval(1, 1.5) = False
Point(1.5) in Interval(1, 2) = True -> (the point 1 is contained in the interval [1,2])
Interval(1, 2) in Interval(1, 2)*Interval(1, 2) = > error (comparing 1 dim with 2 dim)
Point(1.5) in Interval(1, 2)*Interval(1, 2) = > error (looking for a 1 dim point in 2 dim)
Point(1.5, 1.5) in Interval(1, 2)*Interval(1, 2) = True (the point exist in the intervals)

To follow the measure types correctly wold be necessary some changes, like, Point(1) should be the same as Interval(1,1).

I don't know if you will like the idea, but at least i think, should be necessary some order in how sympy works with measure.

What is hard here too is, if is dissabled this feature, how can we know if a point is contained in a interval for example?, at least the "in" function would be less intuitive.

Bye Bye

@asmeurer
Copy link
Member

The error comes from the creation of Point, which fails when there is a single argument. I don't know if it's intended or not. It perhaps is, since I believe the geometry module isn't designed to work with 1-D spaces, but if so, the error message should be improved.

I agree with @sylee957 that it's not a good idea to mix geometric objects and sets. The logical conclusion of allowing this is that Point(1) should be identified with the number 1. A better way to check this would be to create the geometric object you want to check if the point is in, like Segment. If it would be useful, there could be a helper function to convert set objects into geometry objects.

@oscarbenjamin
Copy link
Collaborator

We can also do this in a conceptually valid way using ImageSet:

In [28]: S = ImageSet(Lambda((x, y), Point(x, y)), Interval(0, 2), Interval(0, 2))                                                             

In [29]: S                                                                                                                                     
Out[29]: {Point2D(x, y) | x  [0, 2] , y  [0, 2]}

In [30]: Point(1, 1) in S                                                                                                                      
Out[30]: True

In [31]: Point(1, 3) in S                                                                                                                      
Out[31]: False

@oscarbenjamin
Copy link
Collaborator

As far as I can tell everything here is working fine except that 1D Points don't work.

@czgdp1807
Copy link
Member

Currently, Point2D isn't working too,

isympy
IPython console for SymPy 1.5.dev (Python 3.6.8-64-bit) (ground types: python)

These commands were executed:
>>> from __future__ import division
>>> from sympy import *
>>> x, y, z, t = symbols('x y z t')
>>> k, m, n = symbols('k m n', integer=True)
>>> f, g, h = symbols('f g h', cls=Function)
>>> init_printing()

Documentation can be found at https://docs.sympy.org/dev


In [1]: S = ImageSet(Lambda((x, y), Point(x, y)), 
   ...: Interval(0, 2), Interval(0, 2))           

In [2]: S                                         
Out[2]: {Point2D(x, y) | x ∊ [0, 2] , y ∊ [0, 2]}

In [3]: Point(1, 0) in S                          
--------------------------------------------------
AttributeError   Traceback (most recent call last)
<ipython-input-3-08b0876fc56d> in <module>
----> 1 Point(1, 0) in S

~/sympy/sympy/sets/sets.py in __contains__(self, other)
    637     def __contains__(self, other):
    638         other = sympify(other)
--> 639         c = self._contains(other)
    640         b = tfn[c]
    641         if b is None:

~/sympy/sympy/sets/fancysets.py in _contains(self, other)
    456         variables = tuple(variables)
    457         base_sets = [symsetmap[v] for v in variables]
--> 458         solnset = _solveset_multi(equations, variables, base_sets)
    459         if solnset is None:
    460             return None

~/sympy/sympy/solvers/solveset.py in _solveset_multi(eqs, syms, domains)
   2052             if sym not in eqs[n].free_symbols:
   2053                 continue
-> 2054             sol = solveset(eqs[n], sym, domains[syms.index(sym)])
   2055 
   2056             if isinstance(sol, FiniteSet):

~/sympy/sympy/solvers/solveset.py in solveset(f, symbol, domain)
   1991             try:
   1992                 r = Dummy('r', **assumptions)
-> 1993                 return solveset(f.xreplace({symbol: r}), r, domain
   1994                     ).xreplace({r: symbol})
   1995             except InconsistentAssumptions:

~/sympy/sympy/solvers/solveset.py in solveset(f, symbol, domain)
   2009     f = piecewise_fold(f)
   2010 
-> 2011     return _solveset(f, symbol, domain, _check=True)
   2012 
   2013 

~/sympy/sympy/solvers/solveset.py in _solveset(f, symbol, domain, _check)
    924             result += solns
    925     elif isinstance(f, Eq):
--> 926         result = solver(Add(f.lhs, - f.rhs, evaluate=False), symbol, domain)
    927 
    928     elif f.is_Relational:

~/sympy/sympy/solvers/solveset.py in <lambda>(f, x, domain)
    891 
    892     # assign the solvers to use
--> 893     solver = lambda f, x, domain=domain: _solveset(f, x, domain)
    894     inverter = lambda f, rhs, symbol: _invert(f, rhs, symbol, domain)
    895 

~/sympy/sympy/solvers/solveset.py in _solveset(f, symbol, domain, _check)
    885     elif f.is_Add:
    886         a, h = f.as_independent(symbol)
--> 887         m, h = h.as_independent(symbol, as_Add=False)
    888         if m not in set([S.ComplexInfinity, S.Zero, S.Infinity,
    889                               S.NegativeInfinity]):

AttributeError: 'Point2D' object has no attribute 'as_independent'

In [4]: Point(1, 1) in S                          
--------------------------------------------------
AttributeError   Traceback (most recent call last)
<ipython-input-4-2063532373c8> in <module>
----> 1 Point(1, 1) in S

~/sympy/sympy/sets/sets.py in __contains__(self, other)
    637     def __contains__(self, other):
    638         other = sympify(other)
--> 639         c = self._contains(other)
    640         b = tfn[c]
    641         if b is None:

~/sympy/sympy/sets/fancysets.py in _contains(self, other)
    456         variables = tuple(variables)
    457         base_sets = [symsetmap[v] for v in variables]
--> 458         solnset = _solveset_multi(equations, variables, base_sets)
    459         if solnset is None:
    460             return None

~/sympy/sympy/solvers/solveset.py in _solveset_multi(eqs, syms, domains)
   2052             if sym not in eqs[n].free_symbols:
   2053                 continue
-> 2054             sol = solveset(eqs[n], sym, domains[syms.index(sym)])
   2055 
   2056             if isinstance(sol, FiniteSet):

~/sympy/sympy/solvers/solveset.py in solveset(f, symbol, domain)
   1991             try:
   1992                 r = Dummy('r', **assumptions)
-> 1993                 return solveset(f.xreplace({symbol: r}), r, domain
   1994                     ).xreplace({r: symbol})
   1995             except InconsistentAssumptions:

~/sympy/sympy/solvers/solveset.py in solveset(f, symbol, domain)
   2009     f = piecewise_fold(f)
   2010 
-> 2011     return _solveset(f, symbol, domain, _check=True)
   2012 
   2013 

~/sympy/sympy/solvers/solveset.py in _solveset(f, symbol, domain, _check)
    924             result += solns
    925     elif isinstance(f, Eq):
--> 926         result = solver(Add(f.lhs, - f.rhs, evaluate=False), symbol, domain)
    927 
    928     elif f.is_Relational:

~/sympy/sympy/solvers/solveset.py in <lambda>(f, x, domain)
    891 
    892     # assign the solvers to use
--> 893     solver = lambda f, x, domain=domain: _solveset(f, x, domain)
    894     inverter = lambda f, rhs, symbol: _invert(f, rhs, symbol, domain)
    895 

~/sympy/sympy/solvers/solveset.py in _solveset(f, symbol, domain, _check)
    885     elif f.is_Add:
    886         a, h = f.as_independent(symbol)
--> 887         m, h = h.as_independent(symbol, as_Add=False)
    888         if m not in set([S.ComplexInfinity, S.Zero, S.Infinity,
    889                               S.NegativeInfinity]):

AttributeError: 'Point2D' object has no attribute 'as_independent'

In [5]: Point(1, 3) in S                          
--------------------------------------------------
AttributeError   Traceback (most recent call last)
<ipython-input-5-cae7a6a9a54c> in <module>
----> 1 Point(1, 3) in S

~/sympy/sympy/sets/sets.py in __contains__(self, other)
    637     def __contains__(self, other):
    638         other = sympify(other)
--> 639         c = self._contains(other)
    640         b = tfn[c]
    641         if b is None:

~/sympy/sympy/sets/fancysets.py in _contains(self, other)
    456         variables = tuple(variables)
    457         base_sets = [symsetmap[v] for v in variables]
--> 458         solnset = _solveset_multi(equations, variables, base_sets)
    459         if solnset is None:
    460             return None

~/sympy/sympy/solvers/solveset.py in _solveset_multi(eqs, syms, domains)
   2052             if sym not in eqs[n].free_symbols:
   2053                 continue
-> 2054             sol = solveset(eqs[n], sym, domains[syms.index(sym)])
   2055 
   2056             if isinstance(sol, FiniteSet):

~/sympy/sympy/solvers/solveset.py in solveset(f, symbol, domain)
   1991             try:
   1992                 r = Dummy('r', **assumptions)
-> 1993                 return solveset(f.xreplace({symbol: r}), r, domain
   1994                     ).xreplace({r: symbol})
   1995             except InconsistentAssumptions:

~/sympy/sympy/solvers/solveset.py in solveset(f, symbol, domain)
   2009     f = piecewise_fold(f)
   2010 
-> 2011     return _solveset(f, symbol, domain, _check=True)
   2012 
   2013 

~/sympy/sympy/solvers/solveset.py in _solveset(f, symbol, domain, _check)
    924             result += solns
    925     elif isinstance(f, Eq):
--> 926         result = solver(Add(f.lhs, - f.rhs, evaluate=False), symbol, domain)
    927 
    928     elif f.is_Relational:

~/sympy/sympy/solvers/solveset.py in <lambda>(f, x, domain)
    891 
    892     # assign the solvers to use
--> 893     solver = lambda f, x, domain=domain: _solveset(f, x, domain)
    894     inverter = lambda f, rhs, symbol: _invert(f, rhs, symbol, domain)
    895 

~/sympy/sympy/solvers/solveset.py in _solveset(f, symbol, domain, _check)
    885     elif f.is_Add:
    886         a, h = f.as_independent(symbol)
--> 887         m, h = h.as_independent(symbol, as_Add=False)
    888         if m not in set([S.ComplexInfinity, S.Zero, S.Infinity,
    889                               S.NegativeInfinity]):

AttributeError: 'Point2D' object has no attribute 'as_independent'

@czgdp1807
Copy link
Member

Current status,

>>> Point(1) in Interval(0, 2)
Traceback (most recent call last):
  File "/usr/lib/python3.6/code.py", line 91, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "/home/czgdp1807ssd/sympy_project/sympy/sympy/geometry/point.py", line 126, in __new__
    .format(func_name(coords))))
TypeError: 
Expecting sequence of coordinates, not `int`
>>> Point(1, 1) in Interval(0, 2)*Interval(0, 2)
False

@sylee957
Copy link
Member

sylee957 commented Apr 4, 2020

Now, it looks like Point in one dimensional geometry is not supported.

Actually, I think that the logical problem of equating Point(1) and 1 can be avoided if Point(1) can rather be identified as an element of S.Reals**1 than S.Reals, and in this sense, it can be natural to connect other multidimensional Point with cartesian products of intervals.

@oscarbenjamin
Copy link
Collaborator

We could have an as_tuple method for Point and then it could be identified as an element of S.Reals**1. More generally geometry objects could have an as_tuple_set method and then you would have

>>> Point(1).as_tuple_set()
FiniteSet(Tuple(1))
>>> Point(1).as_tuple_set() < S.Reals**1
True

I don't think we should identify the geometry objects directly as tuples. That kind of confusion around definitions comes up around sympy and always causes problems. It is almost always best just to keep things separate and have clear methods for converting between different types of objects.

@asmeurer
Copy link
Member

asmeurer commented Apr 6, 2020

I agree, although I think it's more natural to convert the set object into a geometric entity rather than converting the point into a number or tuple.

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

No branches or pull requests

7 participants