Added method of intersection for ImageSet with Integer as baseset #7587
Conversation
|
@mrocklin @skirpichev please review. |
| return ImageSet(Lambda(t, f.subs(n, solns[0])), S.Integers) | ||
| else: | ||
| return ImageSet(Lambda(t, g.subs(m, solns[1])), S.Integers) | ||
|
|
There was a problem hiding this comment.
remove everything below
|
@skirpichev @mrocklin I have added method for simplification for |
| def test_infinitely_indexed_set_1(): | ||
| from sympy.abc import n, m, t | ||
| assert imageset(Lambda(n, 2*n), S.Integers).intersect(imageset(Lambda(m, 2*m + 1), S.Integers)) == \ | ||
| EmptySet() |
There was a problem hiding this comment.
What happens if the variables match?
There was a problem hiding this comment.
The current Diophantine method doesn't work then.
There was a problem hiding this comment.
Fixed it in the latest commit.
|
Seems like a neat use of diophantine |
| if solns_set == set(): | ||
| return EmptySet() | ||
| solns = list(diophantine(f - g))[0] | ||
| # XXX: neglecting multiple solutions |
There was a problem hiding this comment.
They can arise is in situations like these.
In [4]: diophantine((n - m)*(n + m))
Out[4]: {(-t, -t), (t, -t)}
I'm not sure if they cannot arise in our use case.
There was a problem hiding this comment.
You shouldn't silently neglect this situation.
|
In general, I'm 👍 for this pr, but I feel it isn't as general as it could be. I think, this should be implemented in the other way: as the _intersect helper for ImageSet's like |
I'm not sure that I understand. That seems like what is implemented here. Not as general as it could be sounds fine to me. People can come and make it more general later. Improving sets isn't really our job for this Summer. |
No. I mean, the intersection between two ImageSet's like |
|
@skirpichev @mrocklin I have squashed all the commits into two. |
|
|
||
| assert imageset(Lambda(m, 2*m), S.Integers).intersect(imageset(Lambda(m, 3*m), S.Integers)) == \ | ||
| ImageSet(Lambda(t, 6*t), S.Integers) | ||
| # XXX: the order of variables decide the sign in the Lambda in the imageset |
There was a problem hiding this comment.
I kept it intentionally so that someone reading the tests understands the reason for different sign in line 184 and line 187
There was a problem hiding this comment.
then move it up and drop XXX (or this does mean something?)
There was a problem hiding this comment.
Done, that. Not sure of the general meaning but I use XXX when I want to say, "warning: ...", "be careful: ...".
The implementation is done in |
| assert imageset(Lambda(n, 2*n), S.Integers).intersect(imageset(Lambda(n, 2*n + 1), S.Integers)) == \ | ||
| EmptySet() | ||
|
|
||
| # the order of variables decide the sign in the Lambda in the imageset |
There was a problem hiding this comment.
I'm not sure if this ok.
n and m - dummy variables, they shouldn't affect the result.
In [1]: hash(Lambda(n, n))
Out[1]: -2984828439234142848
In [2]: hash(Lambda(m, m))
Out[2]: -2984828439234142848
In [3]: hash(Lambda(m, -m)) # but!
Out[3]: 6063053752394237392
There was a problem hiding this comment.
I modified the code so that the result doesn't depend on the variable name, though I couldn't understand your argument given by the hash(Lambda(...)), can you elaborate.
There was a problem hiding this comment.
We have something like Intersection(f, g) in the first case, where f=imageset(Lambda(m, 2*m), S.Integers) and g = imageset(Lambda(n, 3*n), S.Integers), and Intersection(f,h) in the second case, where h = ImageSet(Lambda(m, 3*m), S.Integers), hash(g)=hash(h). All arguments of Intersection have same hashes in both cases. But for rhs - they are different:
In [5]: hash(ImageSet(Lambda(t, -6*t), S.Integers))
Out[5]: 6735991267090917195
In [6]: hash(ImageSet(Lambda(t, 6*t), S.Integers))
Out[6]: 8366958085273728911
There was a problem hiding this comment.
So, you mean if hashes of the input are same, which should imply the input to the function are same then the output should also be the same which wasn't the case because the hashes are not same. I think we should not have hash(Lambda(n, n)) == hash(Lambda(m, m)). That is also the behavior of Python lambda.
In [1]: f = lambda x: x
In [2]: g = lambda y: y
In [3]: hash(f)
Out[3]: -9223372036853107475
In [4]: hash(g)
Out[4]: 1680133
In [5]: hash(f) == hash(g)
Out[5]: False
There was a problem hiding this comment.
On Wed, Jun 18, 2014 at 07:29:59AM -0700, Harsh Gupta wrote:
So, you mean if hashes of the input are same, which should imply the input
to the function are same then the output should also be the same
No. I mean, that the output has different hashes, which imply the input
should be different too. But it isn't:
In [1]: imageset(Lambda(m, 2_m), S.Integers).intersect(imageset(Lambda(n, 3_n), S.Integers)) == imageset(Lambda(m, 2_m), S.Integers).intersect(imageset(Lambda(m, 3_m), S.Integers))
Out[1]: True
I think we should not
have hash(Lambda(n, n)) == hash(Lambda(m, m)). That is also the behavior
of Python lambda.
But Lambda is not a Python's lambda. I don't sure there is a problem.
|
The Travis error is due to timeout. @mrocklin @skirpichev can we merge this now? |
Added method of intersection for ImageSet with Integer as baseset
No description provided.