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

solveset : nonlinsolve - nonlinear system of equation #11111

Merged
merged 28 commits into from Aug 15, 2016

Conversation

Projects
None yet
5 participants
@Shekharrajak
Copy link
Member

Shekharrajak commented May 15, 2016

Non linear system of equation solver.

  • Modify testcases.
  • Non poly cases ( where _solve_poly_system can't help) .
  • Import ALL the testcases from old solve and add new testcases.
  • More clear comments and docstring.
  • Address #11111 (comment)
@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented May 15, 2016

ping @hargup @certik @aktech @kshitij10496 @asmeurer .
Please check this PR . Work is in progress.

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch from 62764a8 to 9621072 May 16, 2016

>>> nlinsolve((e1, e2), (x, y))
>>> {(191/20, -3*sqrt(391)/20), (191/20, 3*sqrt(391)/20)}
"""

This comment has been minimized.

@kshitij10496

kshitij10496 May 16, 2016

Member

The docstrings can be improved.
Here are a few suggestions from my side:

  • a bit more explanation about the method (API) can help
  • few more examples (different types of system) would look cool.

A reference to linsolve might come in handy.

'system is to be found.')
polys = []
failed = []
for j, g in enumerate(system):

This comment has been minimized.

@kshitij10496

kshitij10496 May 16, 2016

Member

I think enumerate might not be an overkill here.

This comment has been minimized.

@kshitij10496

kshitij10496 May 16, 2016

Member

A simpler iteration technique can be used here.

This comment has been minimized.

@Shekharrajak

Shekharrajak May 16, 2016

Author Member

Thanks, I'll modify it in next commit 👍

This comment has been minimized.

@hargup

hargup May 17, 2016

Member

I think enumerate might not be an overkill here.

Just curious, what do you mean by by an 'overkill'?

This method will help in this type of cases(nlinsolve calling this method when
solve_poly_system cannot solve non zero dimensional nonlinear system of equations) :
>>> nlinsolve([(x + y)**2 - 4, x + y - 2],[x,y])
>>> {-y + 2}

This comment has been minimized.

@Shekharrajak

Shekharrajak May 16, 2016

Author Member

here output should be x: -y+2 . If someone knows good way to do this then please suggest.

This comment has been minimized.

@kshitij10496

kshitij10496 May 16, 2016

Member

I think the API should be compatible with that of linsolve.

With that thought in mind, the ideal output should be
{(-y + 2, y)}


def nlinsolve_matrix(system, symbols):
r"""
reference : http://people.math.gatech.edu/~aleykin3/math4803spr13/BOOK/chapter1.pdf

This comment has been minimized.

@hargup

hargup May 17, 2016

Member

Please use the same documentation style as linsolve, first a good description of the method then illustrative examples. Also the references should go the reference section.

return result


def nlinsolve(system, symbols):

This comment has been minimized.

@hargup

hargup May 17, 2016

Member

How is nlinsolve different from nlinsolve_matrix?

This comment has been minimized.

@Shekharrajak

Shekharrajak May 18, 2016

Author Member

nlinsolve_matrix can recognize if system of equation have infinite solutions( Matrix det becomes zero at that time).But now I am thinking about solving positive dimensional system using polynomial univariate representation (PUR) method :

but not getting how to find RUR also called PUR .

This comment has been minimized.

@hargup

hargup May 18, 2016

Member

No, I mean I don't understand why there is a need for two methods nlinsolve and nlinsolve_matrix?

@@ -1012,6 +1012,199 @@ def test_linsolve():
assert linsolve(Augmatrix, a, b, c, d, e) == FiniteSet((a, 0, c, 0, e))


# non linear system of equations

This comment has been minimized.

@hargup

hargup May 17, 2016

Member

Are these tests imported from solve or have you written them yourself?

This comment has been minimized.

@Shekharrajak

Shekharrajak May 18, 2016

Author Member

Most of them are from solve.

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch from 384c875 to e5455c6 May 19, 2016

@Shekharrajak Shekharrajak changed the title solveset : nlinsolve - nonlinear system of equation [WIP] solveset : nlinsolve - nonlinear system of equation May 22, 2016

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch 6 times, most recently from 8022764 to 38a2c7f May 22, 2016

# solved symbols: discard it
# eg : depend_soln=> {x : -y} and
# r1 is function of x like r1 = -x for y
skip = True

This comment has been minimized.

@kshitij10496

kshitij10496 May 23, 2016

Member

Is this line intended to be outside the for block ?

This comment has been minimized.

@Shekharrajak

Shekharrajak May 23, 2016

Author Member

No

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch 12 times, most recently from 3a07cf2 to febb71a May 23, 2016

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented May 25, 2016

In [1]: nlinsolve([exp(x) - sin(y), 1/exp(y) - 3], [x, y]) 
Out[1]: {{x: {ⅈ⋅(2⋅n⋅π + π) + log(sin(log(3))) | n ∊ ℤ}, y: -log(3)}}

In [2]: nlinsolve([exp(x) - sin(y), y**2 - 4], [x, y])
Out[2]: {{x: log(sin(2)), y: 2}, {x: {ⅈ⋅(2⋅n⋅π + π) + log(sin(2)) | n ∊ ℤ}, y: -2}}

In [3]: FiniteSet(Dict(\
   ...:         (x, imageset(Lambda(n, I*(2*n*pi + pi) + log(sin(log(3)))), S.Integers)),\
   ...:          (y, - log(3))))
Out[3]: {{x: {ⅈ⋅(2⋅π⋅n + π) + log(sin(log(3))) | n ∊ ℤ}, y: -log(3)}}

In [4]: FiniteSet(Dict((x, log(sin(S(2)))), (y, S(2))), \
   ...:         Dict((x, imageset(Lambda(n, I*(2*n*pi + pi) + log(sin(2))), S.Integers)),\
   ...:          (y, -2)))
Out[4]: {{x: log(sin(2)), y: 2}, {x: {ⅈ⋅(2⋅π⋅n + π) + log(sin(2)) | n ∊ ℤ}, y: -2}}

When we use SymPy defined dictionary Dict then we need to write testcases like this :

assert nlinsolve([exp(x) - sin(y), 1/exp(y) - 3], [x, y]) == \
    FiniteSet(Dict(\
        (x, imageset(Lambda(n, I*(2*n*pi + pi) + log(sin(log(3)))), S.Integers)),\
         (y, - log(3))))

    assert nlinsolve([exp(x) - sin(y), y**2 - 4], [x, y]) == \
        FiniteSet(Dict((x, log(sin(S(2)))), (y, S(2))), \
        Dict((x, imageset(Lambda(n, I*(2*n*pi + pi) + log(sin(2))), S.Integers)),\
         (y, -2)))

But still these are failing here. I didn't understand why.

Shekharrajak added some commits Jun 21, 2016

minor changes
minor change *symbols
conditionset return, when solveset failed to solve all the eqs
minor changes

minor change

test issue 5132_2 fixed
break into functions
extract_main_soln

hit variable not needed, removed

extract_main_soln returns finiteset

_append_new_soln new func

some new testcases
trig xfail added
minor changes
doctest error fixed
return infinite soln when we have certain soln and general form in result list

symbol sort before groebner

2 soln for doctest positive dim system

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch from 386a364 to c8ab5d3 Aug 5, 2016

eq = eq_unrad
if isinstance(eq, Expr):
eq = eq.as_numer_denom()[0]
# this will remove sqrt if symbols are under it

This comment has been minimized.

@jksuom

jksuom Aug 6, 2016

Member

This will remove numeric roots from the generators but not those that involve symbols.

This comment has been minimized.

@Shekharrajak

Shekharrajak Aug 6, 2016

Author Member

Thanks , actually that comment is not needed.

eq = eq.as_numer_denom()[0]
# this will remove sqrt if symbols are under it
poly = eq.as_poly(*symbols, extension=True)
if poly is not None:

This comment has been minimized.

@jksuom

jksuom Aug 6, 2016

Member

Is poly defined only on the previous line? What if eq is not an expression?

This comment has been minimized.

@Shekharrajak

Shekharrajak Aug 6, 2016

Author Member

I think it should check for Number also, if eq is a Number then eq will not be added in poly and nonpoly list ( since that will not help for finding soln). It seems I forgot to add line poly = None at the starting. I have edited this in new commit. Thanks @jksuom .

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch 2 times, most recently from 54bfdb3 to 69d0578 Aug 6, 2016

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Aug 9, 2016

The coverage report link : https://shekharrajak.github.io/sympy_11111_coverage_report/sympy_solvers_solveset_py.html#n1214

Lines 1378, 1626, 1739 are for adding intersection in final solution. I don't find the testcase for that, but it is similar to adding complement (line 1382), so it will work correctly.

Line 2023, _solve_poly_system works well(I don't have testcase, when it fails). If it fails then substitution can handle that system(I have checked it manually). Similarly line 1634.

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch from 69d0578 to 804db87 Aug 9, 2016

@@ -1309,22 +1309,24 @@ def substitution(system, symbols, result=[{}], known_symbols=[],
return S.EmptySet

if not symbols:
raise ValueError('Symbols must be given, for which solution of the '
'system is to be found.')
msg = 'Symbols must be given, for which solution of the '\

This comment has been minimized.

@aktech

aktech Aug 13, 2016

Member

Simply do this:

msg = ('Symbols must be given, for which solution of the '
       'system is to be found.')

Avoid using \

raise ValueError(filldedent(msg))

if not is_sequence(symbols):
msg = 'symbols should be given as a sequence, e.g. a list.' \

This comment has been minimized.

@aktech

aktech Aug 13, 2016

Member

same here

if not is_sequence(symbols):
raise TypeError(
'syms should be given as a sequence, e.g. a list')
msg = 'Iterable of symbols must be given as' \

This comment has been minimized.

@aktech

aktech Aug 13, 2016

Member

same here

raise ValueError('Symbols or iterable of symbols must be given as '
'second argument, not type \
%s: %s' % (type(symbols[0]), symbols[0]))
msg = 'Symbols or iterable of symbols must be given as \

This comment has been minimized.

@aktech
@aktech

This comment has been minimized.

Copy link
Member

aktech commented Aug 13, 2016

Apart from the couple of minor comments, It looks fine to me. We can push it for now and improve later. @Shekharrajak Thanks for your work.

Will push in 24hr after the minor comments are addressed, if there is no objection.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Aug 13, 2016

Thanks for review.

minor changes
missing space added

@Shekharrajak Shekharrajak force-pushed the Shekharrajak:gsoc_nlinsolve branch from 7d18bb2 to fbdc2be Aug 15, 2016

@aktech aktech merged commit ae54e74 into sympy:master Aug 15, 2016

1 check passed

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

This comment has been minimized.

Copy link
Member

aktech commented Aug 15, 2016

@Shekharrajak Thanks for working on it. Its in.

@Shekharrajak

This comment has been minimized.

Copy link
Member Author

Shekharrajak commented Aug 16, 2016

Thanks.

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.