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

diophantine: permute signs to get missing solutions #11334

Merged
merged 9 commits into from Dec 16, 2016

Conversation

Projects
None yet
7 participants
@Shekharrajak
Copy link
Member

Shekharrajak commented Jul 1, 2016

permute_sign is used when all the variables of the diophantine eq have even power and other terms ( like x, y, z, x_y , y_z, z*x, ..) are not present.

Master branch

In [1]: from sympy.abc import a, b, c, d, e, f

In [2]: eq = a**4 + b**4 - (2**4 + 3**4)

In [3]: diophantine(eq)
Out[3]: set([(2, 3)])

In [4]: eq = a**2 + b**2 + c**2 + d**2 + e**2 - 234
In [5]: len(diophantine(eq))
Out[5]: 35

This branch

In [1]: from sympy.abc import a, b, c, d, e, f

In [2]: eq = a**4 + b**4 - (2**4 + 3**4)

In [3]: diophantine(eq, permute=True)
Out[3]: set([(-3, -2), (-3, 2), (-2, -3), (-2, 3), (2, -3), (2, 3), (3, -2), (3, 2)])

In [4]: eq = a**2 + b**2 + c**2 + d**2 + e**2 - 234
In [5]: len(diophantine(eq, permute=True))
Out[5]:  62000
@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 1, 2016

Parameter permute_sign is added. Default value is False.
I added this parameter, because if diophantine eq is something like
eq = a**2 + b**2 + c**2 + d**2 + e**2 - 234 (general_sum_of_squares or something like that) then it takes bit more time, because it has large number is solution. So if user is not interested in permute sign then they can skip.
I am not sure whether it is a good idea or not. Suggestions are welcome.

# here var_mul is like [(x, y)]
check_coeff = True
x, y = symbols('x, y')
global map

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Jul 1, 2016

Author Member

Without this line (global map). I got unboundlocalerror : map local variable referenced before assignment. If someone knows find the error then let me know.

This comment has been minimized.

Copy link
@gxyd

gxyd Jul 1, 2016

Member

Since map keyword has already been assigned here
https://github.com/Shekharrajak/sympy/blob/4e80f22ee715433a4e9512cc2b02e266283082ea/sympy/solvers/diophantine.py#L160
. Perhaps that name "map" should better be changed. It is not wise to
use global keyword, I guess.

On 07/01/2016 05:11 PM, Shekhar Prasad Rajak wrote:

In sympy/solvers/diophantine.py
#11334 (comment):

  •                        coefficint = c[v1_mul_v2]
    
  •                    except KeyError:
    
  •                        coefficint = 0
    
  •                    # if coeff(y_z), coeff(y_x), coeff(x*z) is not 0 then
    
  •                    # check_coeff will be True and do_permute_sign will
    
  •                    #  remain False.
    
  •                    check_coeff = bool(check_coeff & coefficint)
    
  •                if not check_coeff:
    
  •                    # means only x**2, y**2, z**2, const is present
    
  •                    do_permute_signs = True
    
  •            elif len_var == 2:
    
  •                var_mul = list(subsets(v, 2))
    
  •                # here var_mul is like [(x, y)]
    
  •                check_coeff = True
    
  •                x, y = symbols('x, y')
    
  •                global map
    

Without this line (|global map|). I got |unboundlocalerror| : map
local variable referenced before assignment. If someone knows find the
error then let me know.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/sympy/sympy/pull/11334/files/4e80f22ee715433a4e9512cc2b02e266283082ea#r69287067,
or mute the thread
https://github.com/notifications/unsubscribe/AHWt8YVKg978jNI0eUAco2mUibkRAs7Nks5qRPzngaJpZM4JDDvX.

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Jul 1, 2016

Author Member

Yes, I wanted to remove global keyword. global keyword works but also shows warning.
Thanks, I need to change the variable name there.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 1, 2016

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jul 1, 2016

Why is it False by default?

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 1, 2016

@asmeurer , I am not sure whether it should be False or True. permute_sign is needed when all the variables are with even power and some const (other terms like x_y, y_z, z*x should not be present).
So if user knows this then user can do permute_sign=True. I am thinking to use permute_sign=True when we call diophantine in solveset_integers #11234 .

If it is not a good idea I'll remove permute_sign parameter.

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jul 1, 2016

It just seems to me that it should be done automatically in diophantine for the equations that it is valid for. Regarding solveset_integers, I'd say all the actual solving logic should go in diophantine, and solveset_integers should only be concerned with converting things to sets, and noting which solvers return complete solutions.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 2, 2016

@asmeurer Thanks.

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch from db47e19 to 20c90f2 Jul 2, 2016

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Jul 2, 2016

I guess I could be in favor of this as long as the ability to get the solution without sign permutation remains (in this case power_representation is the lowest level function that gives the base solution). Part of the reason I stayed away is that once you say that you are going to return sign permutations then why not permutations? Example of the latter are given in the power_representation docstring. I envisioned that solveset would be the one to return all solutions while diophantine would be a lower level solution finder requiring a little more insight from the user (e.g. realizing that anything with even powers could entertain negative solutions, too).

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 3, 2016

Thanks @smichr .

  • power_representation is used in when diop_type is general_sum_of_squares or
    general_sum_of_even_powers. If we really want to permute sign and values as well then we can use signed_permutations.
>>> list(signed_permutations((1, 12)))
    [(1, 12), (-1, 12), (1, -12), (-1, -12), (12, 1), (-12, 1), (12, -1), (-12, -1)]
  • If diop_type is general_pythagorean then only permute_signs is needed .
    >>> list(permute_signs((1, 12)))
    [(1, 12), (-1, 12), (1, -12), (-1, -12)]
  • If diop_type is homogeneous_ternary_quadratic or homogeneous_ternary_quadratic_normal or binary_quadratic then after checking (all/some variables are present in eq with even powers and other terms like x_y, y_z, z*x etc should not be there) we can do permute_signs.
  • power_representation(n, p, k, zeros=False) Represent non-negative number
    n as a sum of k pth powers also it is for base solution. So no need of internal changes.

Please share your view on this.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 4, 2016

Some additional points :

  1. general_sum_of_even_powers or general_sum_of_squares equation are like x_{1}^e + x_{2}^e + . . . + x_{n}^e - k = 0 where e is an even, integer power. So there is no coeff (all are one) then we can do signed_permutations (variable soln and sign permutation).
  2. general pythagorean equation is a_{1}^2x_{1}^2 + a_{2}^2x_{2}^2 + . . . + a_{n}^2x_{n}^2 - a_{n + 1}^2x_{n + 1}^2 = 0. Here coeff is present so so base soln can't be permuted, we can only permute the sign. Only need permute_signs.
  3. homogeneous_ternary_quadratic, homogeneous_ternary_quadratic_normal : general quadratic ternary form, ax^2 + by^2 + cz^2 + fxy + gyz + hxz = 0. So if coeff of x*y, y*z, z*x are zero, then do
    permute_sign (since we have coeff a, b, c for x**2, y**2, z**2 so use of signed_permutations will be incorrect).
    similarly for quadratic diophantine equations i.e. equations of the form Ax^2 + Bxy + Cy^2 + Dx + Ey + F = 0.
    Actually if coeff of x*y, y*z, z*x, x, y.. are zero, then it may be of the form 1(general_sum_of_even_powers) or 2 (pythagorean) but diophantine use factor_list so binary quad eq like x**2 - y**2 will be solved through (x - y) , (x + y) which are linear. So at the end we can permute the sign. (I don't know the testcase for this 3rd type right now).
@smichr

This comment has been minimized.

Copy link
Member

smichr commented Jul 4, 2016

general Pythagorean...base soln can't be permuted

All variables with the same coefficient would have their solutions permuted: for 2a^2 + 2b^2 + 3c^2 = C the solutions for a and be would be permuted. Basically, for any equation where an exchange of variables gives the same equation, the solutions for those variables would be permuted.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 4, 2016

Thanks @smichr , I need to look in this matter. I saw the code diop_general_pythagorean, is it already confirmed that we will get parametrized solution in terms of m1, m2, ...?

ping @thilinarmtb

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch from 9225b79 to defb60f Jul 5, 2016

@jksuom

This comment has been minimized.

Copy link
Member

jksuom commented Jul 18, 2016

158 commits?

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch from 1439f5b to 9e417ba Jul 18, 2016

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Jul 18, 2016

Sorry, I did add, commit (instead of add) to fix rebase conflict. It is fixed now.

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch 2 times, most recently from c608adb to 6af46fc Jul 18, 2016

@@ -93,7 +93,8 @@ def _even(i):
return i % 2 == 0


def diophantine(eq, param=symbols("t", integer=True), syms=None):
def diophantine(eq, param=symbols("t", integer=True), syms=None,
base_soln=True):

This comment has been minimized.

Copy link
@smichr

smichr Jul 21, 2016

Member

This should be described in the docstring.

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Jul 22, 2016

Author Member

I have added few line in this commit

Thanks.

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Jul 30, 2016

Author Member

@smichr ,Let me know if something else is expected.

var1_mul_var2 = map(lambda a, b: a * b, var_mul)
for v1_mul_v2 in var1_mul_var2:
try:
coefficint = c[v1_mul_v2]

This comment has been minimized.

Copy link
@smichr

smichr Jul 21, 2016

Member

sp: coefficient

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Jul 22, 2016

Author Member

Edited thanks.

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch from 7b94014 to dc4cfb1 Jul 21, 2016

See Also
========
diop_solve()
"""

from sympy.utilities.iterables import (

This comment has been minimized.

Copy link
@kshitij10496

kshitij10496 Aug 4, 2016

Member

I think this import can be made prettier.

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Aug 5, 2016

Author Member

I will write these in one line, but PEP plugin doesn't show error when you write like this.

'binary_quadratic']
if t in permute_signs_for:
do_permute_signs_var = True
elif t in (permute_signs_check):

This comment has been minimized.

Copy link
@kshitij10496

kshitij10496 Aug 4, 2016

Member

We can get rid of the parentheses here.

# here var_mul is like [(x, y), (x, z), (y, z)]
check_coeff = True
a, b = symbols('a, b')
var1_mul_var2 = map(lambda a, b: a * b, var_mul)

This comment has been minimized.

Copy link
@kshitij10496

kshitij10496 Aug 4, 2016

Member

Can we remove the spaces around * ?

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Aug 5, 2016

Author Member

PEP plugin shows error if there is no space around operator, so I used it.

This comment has been minimized.

Copy link
@hargup

hargup Aug 5, 2016

Member

In SymPy we don't follow that convention

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch from 7ad2bcf to d56c641 Aug 5, 2016

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Aug 5, 2016

I think this PR is good to go. Ping @smichr .

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Aug 21, 2016

These changes will not effect previous implementation. Ping @smichr .

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Sep 3, 2016

Does this need updating with master? I got the following wrong answer:

>>> diophantine(x**2+y**4-1**2-3**4)
set([(1, 9)])

In master I get

>>> diophantine(x**2+y**4-1**2-3**4)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "sympy\solvers\diophantine.py", line 197, in diophantine
    var_t, _, eq_type = classify_diop(base, _dict=False)
  File "sympy\solvers\diophantine.py", line 423, in classify_diop
    diop_classify().'''))
NotImplementedError:
This equation is not yet recognized or else has not been simplified
sufficiently to put it in a form recognized by diop_classify().
>>>
@smichr

This comment has been minimized.

Copy link
Member

smichr commented Sep 3, 2016

It seems like the following should come back as set([(1, 9)])

>>> diophantine(x**2+y**2-1**2-3**4)
set([(-1, 9), (-1, -9), (1, 9), (1, -9)])

If it did, it would be more consistent with how even powers behave:

>>> diophantine(x**4+y**4-1**2-3**4)
set([(1, 3)])
>>> diophantine(x**4+y**4-1**2-3**4, base_soln=False)
set([(1, 3), (-1, 3), (3, 1), (-1, -3), (-3, 1), (3, -1), (1, -3), (-3, -1)])

Also (minor suggestion), my preference would be for a postive keyword like permute=True instead of negative like base_soln=False.

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch 2 times, most recently from cc05def to 8dd1bd5 Sep 4, 2016

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_diophantine_even_powers branch from 8dd1bd5 to e69958f Sep 4, 2016

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Sep 4, 2016

@smichr

Does this need updating with master? I got the following wrong answer:

That is fixed in recent PR, I have updated this PR branch(rebase with master).

It seems like the following should come back as set([(1, 9)])

Now it returns

In [1]: diophantine(x**2+y**2-1**2-3**4)
Out[1]: set([(1, 9)])

In [2]: diophantine(x**2+y**2-1**2-3**4, permute=true)
Out[2]: set([(-1, -9), (-1, 9), (1, -9), (1, 9)])

In 2nd caseset([(-9, -1), (-9, 1), (9, -1), (9, 1)]) is also ans. I am working on it.

@@ -117,6 +118,14 @@ def diophantine(eq, param=symbols("t", integer=True), syms=None):
``t`` is the optional parameter to be used by ``diop_solve()``.
``syms`` is an optional list of symbols which determines the
order of the elements in the returned tuple.
By default, base solution will be returned. If ``permute`` is set to

This comment has been minimized.

Copy link
@smichr

smichr Sep 5, 2016

Member

Suggested rewrite (catching some typos along the way):

By default, only the base solution is returned. If ``permute`` is set to
True then permutations of the base solution and/or permutations of the
signs of the values will be returned when applicable.

>>> eq = a^4 + b^4 - (2^4 + 3^4)
>>> diophantine(eq)
set([2, 3)]
>>> diophantine(eq, permute=True)
set([(([(-3, -2), (-3, 2), (-2, -3), (-2, 3), (2, -3), (2, 3), (3, -2), (3, 2)])

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Sep 25, 2016

Author Member

Thanks, I have edited the docstring.

@@ -136,12 +145,22 @@ def diophantine(eq, param=symbols("t", integer=True), syms=None):
set([(0, n1, n2), (t_0, t_1, 2*t_0 + 3*t_1)])
>>> diophantine(x**2 + 3*x*y + 4*x)
set([(0, n1), (3*t_0 - 4, -t_0)])
>>> from sympy import symbols
>>> a, b = symbols('a, b')

This comment has been minimized.

Copy link
@smichr

smichr Sep 5, 2016

Member

move to docstring above as suggested?

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Sep 25, 2016

Author Member

Done.

>>> diophantine(a**4 + b**4 - (2**4 + 3**4))
set([(2, 3)])
>>> diophantine(a**4 + b**4 - (2**4 + 3**4), permute=True)
set([(-3, -2), (-3, 2), (-2, -3), (-2, 3), (2, -3), (2, 3), (3, -2), (3, 2)])
See Also
========
diop_solve()

This comment has been minimized.

Copy link
@smichr

smichr Sep 5, 2016

Member

reference the two permute functions here?

This comment has been minimized.

Copy link
@Shekharrajak

Shekharrajak Sep 25, 2016

Author Member

Done.

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Sep 5, 2016

Looking good. I added a few more comments.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Sep 25, 2016

Thanks @smichr for the review. I have edited the PR as suggested. Sorry for late reply.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Oct 15, 2016

Ping @smichr

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Nov 3, 2016

I think this PR is good to go.

@smichr

This comment has been minimized.

Copy link
Member

smichr commented Dec 16, 2016

OK, looks good. This is in.

@smichr smichr merged commit dfa7387 into sympy:master Dec 16, 2016

1 check passed

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

@jksuom jksuom referenced this pull request Dec 27, 2016

Merged

diophantine cornacchia v2 #11989

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.