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

[WIP][GSOC] Added _solve_modular for handling equations a - Mod(b, c) = 0 where only b is expr #16976

Open
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@jmig5776
Copy link
Member

commented Jun 5, 2019

References to other Issues or PRs

Brief description of what is fixed or changed

This PR adds a solver for _solve_modular in solveset for handling equations a - Mod(b, c) = 0 where b is Expr.
Examples :

>>>solveset(3 - Mod(5*x - 8, 7), x)
ImageSet(Lambda(n, 7*n + 5), S.Integers)

Also closes #13178

Other comments

@aktech @Yathartha22 @Shekharrajak

Release Notes

  • solvers
    • added _solve_modular to handle equations of type a - mod(b, c) = 0 where b is expr
@sympy-bot

This comment has been minimized.

Copy link

commented Jun 5, 2019

Hi, I am the SymPy bot (v147). 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
    • added _solve_modular to handle equations of type a - mod(b, c) = 0 where b is expr (#16976 by @jmig5776)

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

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.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234". See
https://github.com/blog/1506-closing-issues-via-pull-requests . Please also
write a comment on that issue linking back to this pull request once it is
open. -->


#### Brief description of what is fixed or changed
This PR adds a solver for `_solve_modular` in `solveset` for handling equations a - Mod(b, c) = 0 where b is Expr.
Examples :
```python
>>>solveset(3 - Mod(5*x - 8, 7), x)
ImageSet(Lambda(n, 7*n + 5), S.Integers)
```
Also closes #13178

#### Other comments
@aktech @Yathartha22 @Shekharrajak 

#### Release Notes

<!-- Write the release notes for this release below. See
https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more information
on how to write release notes. The bot will check your release notes
automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* solvers
  * added `_solve_modular` to handle equations of type a - mod(b, c) = 0 where b is expr
<!-- END RELEASE NOTES -->

@codecov

This comment has been minimized.

Copy link

commented Jun 5, 2019

Codecov Report

Merging #16976 into master will increase coverage by 0.039%.
The diff coverage is 85.714%.

@@              Coverage Diff              @@
##            master    #16976       +/-   ##
=============================================
+ Coverage   73.943%   73.982%   +0.039%     
=============================================
  Files          620       620               
  Lines       160403    160541      +138     
  Branches     37635     37667       +32     
=============================================
+ Hits        118608    118773      +165     
+ Misses       36306     36286       -20     
+ Partials      5489      5482        -7
@jmig5776

This comment has been minimized.

Copy link
Member Author

commented Jun 5, 2019

Soon I will be adding proof of correctness and code for increasing more equations.

@jmig5776

This comment has been minimized.

Copy link
Member Author

commented Jun 6, 2019

Motivation of this PR

Currently solveset is unable to handle modular equations i.e returns ConditionSet. So to handle modular equations I am trying to implement a new solver for it named _solve_modular. It will be handling following type of equations.

A - Mod(B, C) = 0
where A, B are functions of x 

Now to achieve this I had separated out some individual goals as:

  1. Where A is function of x and B is constant. ---> This case is already handled by sympy beacuse while simplifying the equation Mod(B, C) will be already evaluated.
  2. Where A is constant and B is function of x. ----> This PR will handle those cases.
  3. Where A and B both are functions of x. ----> This will be implemented after properly discussing with mentors.

Explanation of this PR

As in this PR I am considering case where A is constant and B is function of x.

  • Separating out equations involving mod and of proper type as we require
    For that I had define is_modular which checks required conditions.
  • Defining a new solver _solve_modular to handle those equations
    Arguments provided to _solve_modular are mainly rhs and modterm .

How does _solve_modular handles equation

A - Mod(B, C) = 0
As B is function of x so the answer is mainly calculated by solving

B - C*n - A = 0
  • when B is x then C*n + A is the required solution
  • when B is instance of Add then we try to find two independent parts of B and the symbol independent gets tranferred to other side and again the solveset is called on the symbol dependent part.
  • when B is instance of Mul then same as we done in Add we separate out the dependent and independent parts and tranfer the independent part to rhs using invert here.
  • when B is instance of Pow i.e B = base**exp then 3 cases arise as following
  1. base is function of x and exp is constant.
  2. base is constant and exp is function of x.
  3. base and exp both function of x.
    Cases 1 and 3 can be handled by solving equation B - C*n - A = 0.
    Case 2 is handled by first taking discrete log of the base and then again applying solveset to new equation.

Examples handled by this solver

>>> from sympy.solvers.solveset import _solve_modular as solve_modulo
>>> from sympy import S, Symbol, sin
>>> from sympy.core.mod import Mod
>>> x = Symbol('x')
>>> solve_modulo(Mod(5*x - 8, 7), 3, x, S.Reals) # Earlier returns ConditionSet
ImageSet(Lambda(_n, 7*_n + 5), Integers)
>>> solve_modulo(Mod(x**2 - 2*x + 1, 7), 3, x, S.Reals) # earlier returns ConditionSet
ImageSet(Lambda(_n, {1 - sqrt(7*_n + 3), sqrt(7*_n + 3) + 1}), Integers)
>>> solve_modulo(Mod(sin(x), 7), 3, x, S.Reals) # not solvable specifically
ImageSet(Lambda(_n, ConditionSet(x, Eq(-7*_n + sin(x) - 3, 0), Reals)), Integers)

It also solves this issue #13178

>>> n = symbols('n', integer=True)
>>> a = 742938285
>>> z = 1898888478
>>> m = 2**31 - 1
>>> x = 20170816
>>> solveset(x - Mod(a**n*z, m), n, S.Integers)
FiniteSet(100)
@Yathartha22
Copy link
Contributor

left a comment

I have left a few comments below. I haven't checked all the tests will look at it afterwards.
Couple of things to note:

  • The result should be according to what the domain is specified. I tried few tests:
>>> solveset(3 - Mod(5*x, 7), x, Interval(0, 10))
ImageSet(Lambda(_n, 7*_n + 2), Integers)    # should be {2, 9} 
>>> solveset(3 - Mod(5*x - 8, 7), x, Interval(0, 10))
ConditionSet(x, Eq(Mod(5*x + 6, 7) - 3, 0), Interval(0, 10))    # should be {5}
  • I also think that the flow of _solve_modular should be changed primarily because there are a lot of recursive calls to _solveset which ultimately gets into _solve_modular itself (which may slightly increase computation time), so if _solve_modular is acting as an independent function it should not call _solveset. Maybe we can have an inverter within _solve_modular, which recursively does the work what currently the solver does.
if _is_modular(....):
    return _solve_modular(....)

def _solve_modular(....):
    ....
    ....
    # some preprocessing ...
    .... 
    ....
    lhs, rhs = _invert_modular(....)
    if lhs == symbol:     # as of now we expect inverter to perform complete inversion
        return rhs
    else:
        return ConditionSet(....)

Something of this sought will be better, atleast I think so 😄 . This is not exactly what the design should be like, you can manipulate accordingly. My motive is to let you understand the overview of the design. Feel free to add your suggestions.

# modular tests
def test_solve_modular():
x = Symbol('x')
n = Dummy('n', real=True)

This comment has been minimized.

Copy link
@Yathartha22

Yathartha22 Jun 24, 2019

Contributor

I don't think this should be defined real. Keep this as vanilla. n needs to be an integer which will be taken care in the second argument of imageset.

g_ys = solver(f , t, S.Integers)
if isinstance(g_ys, FiniteSet):
for rhs in g_ys:
if not rhs.has(symbol) and modterm.has(symbol):

This comment has been minimized.

Copy link
@Yathartha22

Yathartha22 Jun 24, 2019

Contributor

No need to check again whether modterm.has(symbol). Also add all checks in is_modular itself, it should return True to all those equations that are solvable by the solver (which you should mention in the documentation). Once the equation is passed by is_modular there should be no further checks.

@@ -931,6 +934,15 @@ def _solveset(f, symbol, domain, _check=False):
except NotImplementedError:
result = ConditionSet(symbol, f, domain)
return result
elif f.has(Mod) and is_modular(f, symbol):

This comment has been minimized.

Copy link
@Yathartha22

Yathartha22 Jun 24, 2019

Contributor

f.has(Mod) should go within is_modular. It should be simply:

...
elif is_modular(f, symbol):
    ...
    ...
    result = _solve_modular(...)
    return result
@@ -998,6 +1010,91 @@ def _solveset(f, symbol, domain, _check=False):

return result

def is_modular(f, symbol):

This comment has been minimized.

Copy link
@Yathartha22

Yathartha22 Jun 24, 2019

Contributor

Make this as a private routine.

"""
a, m = modterm.args
n = Dummy('n', real=True)

This comment has been minimized.

Copy link
@Yathartha22

Yathartha22 Jun 24, 2019

Contributor

Make it vanilla.

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.