extended solveset for polynomial congruence #18402

abhinav-anand-addepar · 2020-01-19T21:30:46Z

solvers
- Extended solvers to solve polynomial congruence equation when the domain is subset of integers

sympy-bot · 2020-01-19T21:30:56Z

✅

Hi, I am the SymPy bot (v149). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

solvers
- Extended solvers to solve polynomial congruence equation when the domain is subset of integers (#18402 by @abhinav28071999)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- BEGIN RELEASE NOTES -->
* solvers
  * Extended solvers to solve polynomial congruence equation when the domain is subset of integers
<!-- END RELEASE NOTES -->

abhinav-anand-addepar · 2020-01-19T21:32:30Z

@smichr @oscarbenjamin This might not be the best way and it need improvements.

oscarbenjamin · 2020-01-20T23:42:09Z

I don't know this code so well. Maybe @jmig5776 does...

jmig5776 · 2020-01-21T19:47:01Z

sympy/solvers/solveset.py

@@ -1175,7 +1175,17 @@ def _invert_modular(modterm, rhs, n, symbol):
        if g is not S.One:
            x_indep_term = rhs*invert(g, m)
            return _invert_modular(Mod(h, m), Mod(x_indep_term, m), n, symbol)
-
+    if a.is_polynomial():


@abhinav28071999 The main problem with this PR is that you code added in _invert_modular does not fulfill the goal of _invert_modular.
Let me explain it briefly.
_invert_modular is supposed to return the inverted form of the equation so that the solution of inverted form should give all solutions of the original equation. And here it is missing.

jmig5776 · 2020-01-21T19:55:22Z

sympy/solvers/tests/test_solveset.py

@@ -2172,12 +2172,12 @@ def test_invert_modular():
    assert invert_modular(Mod(x + 8, 7), S(5), n, x) == \
            (x, ImageSet(Lambda(n, 7*n + 4), S.Integers))
    assert invert_modular(Mod(x**2 + x, 7), S(5), n, x) == \
-            (Mod(x**2 + x, 7), 5)
+            (x, ImageSet(Lambda(n, 7*n + 3), S.Integers))


Let us take this example According to your code
7*n+3 is *all the solutions to Mod(x**2 + x, 7)-5.
But that's not the case :
Clearly solution of x**2 + x - 5=0 gives one of the solution to this equation.
i.e x = (-1+/- sqrt(21))/2 which is not included in your solution.
I hope you understand the crux of the problem. Try to be creative to solve this problem. Its nice you are working on this. Try to add it in solve_modular.

Yes you are right, i get it now. I should implement it to solve_modular function which is designed to return set of integer solutions.
It would be very difficult to invert a general polynomial modular equation to general solutions, but it is relatively easier to invert it to only integer solutions.

@jmig5776 One more thing, the current implementation of invert_modular does not seem to give all solution.
For example: invert_modular(Mod(x**4, 7), S(5), n, x) output (x, EmptySet) . But 5**0.25 is a solution to Mod(x**4,7) -5.

Yeah this bug was irritating while we were creating _invert_modular. Can you try to fix this? Although this is in the further development task of solve_modular.

@smichr @jmig5776 I don't think that this bug can be solved for a general polynomial. For example: mod(x**2+x-5, 10) =0, integer solution can be easily calculated but for other solution we have to solve for roots of x**2+x-5 = 10k where k is any integer. For f(x)=x**n it may be done but for general polynomial this will not be feasible as there will be no relation between the roots for different k. Moreover _invert_modular function is only used by _solve_modular, and the return type of _solve_modular is set of integers solutions so this is suitable for _solve_modular but not for _invert_modular.
For now the changes i have made should do the trick for solving polynomial congruence equation. What are your thoughts on this?

abhinav-anand-addepar · 2020-01-21T21:05:10Z

@jmig5776 I made changes to the solve_modular function. This might not be the best way. It is impossible to invert a general polynomial modular equation to its all roots, but it is simpler to get its integer solutions.

codecov · 2020-01-22T00:41:12Z

Codecov Report

Merging #18402 into master will increase coverage by 0.093%.
The diff coverage is 100%.

@@              Coverage Diff              @@
##            master    #18402       +/-   ##
=============================================
+ Coverage   75.292%   75.386%   +0.093%     
=============================================
  Files          633       637        +4     
  Lines       166944    167355      +411     
  Branches     39396     39595      +199     
=============================================
+ Hits        125697    126163      +466     
+ Misses       35709     35665       -44     
+ Partials      5538      5527       -11

smichr · 2020-01-22T01:36:53Z

but it is simpler to get its integer solutions.

Then you can use this when the domain is a subset of Integers.

jmig5776 · 2020-01-22T12:19:39Z

Add some more description to the PR.

Then you can use this when the domain is a subset of Integers.
Consider this comment by @smichr

czgdp1807 · 2020-02-19T18:13:29Z

@abhinav28071999 Any updates on this?

abhinav-anand-addepar · 2020-02-19T18:23:53Z

@smichr @czgdp1807 @jmig5776 Based on #18402 (comment) I think that when the polynomial equation is used the invert modular inverts the equation only in integer domain. More over modular equations are only meaningful in subset of integer domain. So probably the definition of _invert_modular function should be changed.

czgdp1807 · 2020-05-09T17:28:12Z

@jmig5776 @smichr Any thoughts on #18402 (comment) ?

jmig5776 · 2020-05-09T21:04:10Z

I agree with you that it is extremely difficult to generalize the _invert_modular. You should open a PR for inverting only in Integer domain.
But I would like to know what @smichr has to say about this?

extended solveset for polynomial congruence

0914842

abhinav-anand-addepar requested review from jmig5776 and aktech and removed request for aktech January 19, 2020 21:32

minor changes

4685047

czgdp1807 added solvers solvers.solveset labels Jan 20, 2020

abhinav-anand-addepar added 2 commits January 21, 2020 22:49

corrected minor mistakes

4bca2f3

fixed doc test

dfb13a7

jmig5776 reviewed Jan 21, 2020

View reviewed changes

abhinav-anand-addepar added 2 commits January 22, 2020 02:29

made changes in solve_modular function

b258cea

changed doc test

be6cf13

czgdp1807 added the Please take over label Jan 16, 2021

github-actions bot added the Merge conflict label Oct 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

extended solveset for polynomial congruence #18402

extended solveset for polynomial congruence #18402

abhinav-anand-addepar commented Jan 19, 2020 •

edited

sympy-bot commented Jan 19, 2020 •

edited

abhinav-anand-addepar commented Jan 19, 2020

oscarbenjamin commented Jan 20, 2020

jmig5776 Jan 21, 2020

jmig5776 Jan 21, 2020

abhinav-anand-addepar Jan 21, 2020

abhinav-anand-addepar Jan 21, 2020 •

edited

jmig5776 Jan 22, 2020

abhinav-anand-addepar Jan 22, 2020

abhinav-anand-addepar commented Jan 21, 2020

codecov bot commented Jan 22, 2020

smichr commented Jan 22, 2020

jmig5776 commented Jan 22, 2020

czgdp1807 commented Feb 19, 2020

abhinav-anand-addepar commented Feb 19, 2020

czgdp1807 commented May 9, 2020

jmig5776 commented May 9, 2020

extended solveset for polynomial congruence #18402

Are you sure you want to change the base?

extended solveset for polynomial congruence #18402

Conversation

abhinav-anand-addepar commented Jan 19, 2020 • edited

sympy-bot commented Jan 19, 2020 • edited

abhinav-anand-addepar commented Jan 19, 2020

oscarbenjamin commented Jan 20, 2020

jmig5776 Jan 21, 2020

Choose a reason for hiding this comment

jmig5776 Jan 21, 2020

Choose a reason for hiding this comment

abhinav-anand-addepar Jan 21, 2020

Choose a reason for hiding this comment

abhinav-anand-addepar Jan 21, 2020 • edited

Choose a reason for hiding this comment

jmig5776 Jan 22, 2020

Choose a reason for hiding this comment

abhinav-anand-addepar Jan 22, 2020

Choose a reason for hiding this comment

abhinav-anand-addepar commented Jan 21, 2020

codecov bot commented Jan 22, 2020

Codecov Report

smichr commented Jan 22, 2020

jmig5776 commented Jan 22, 2020

czgdp1807 commented Feb 19, 2020

abhinav-anand-addepar commented Feb 19, 2020

czgdp1807 commented May 9, 2020

jmig5776 commented May 9, 2020

abhinav-anand-addepar commented Jan 19, 2020 •

edited

sympy-bot commented Jan 19, 2020 •

edited

abhinav-anand-addepar Jan 21, 2020 •

edited