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

Rubi ruleset using MatchPy (Python 3.6 only) #12978

Merged
merged 228 commits into from Sep 2, 2017

Conversation

Projects
None yet
6 participants
@Upabjojr
Contributor

Upabjojr commented Jul 16, 2017

No description provided.

parsoyaarihant and others added some commits Jun 6, 2017

Abdullahjavednesar and others added some commits Aug 22, 2017

added all rules into Rubi
Added all rules into rubi_intergate().

Some Mathematica Special functions are not yet defined, Hence I have not imported Special functions

There are some parsing issues with derivative rules. I will add derivative rules as soon as the parser is fixed.
Show outdated Hide outdated sympy/integrals/rubi/utility_function.py
if len(match) == len(keys):
a, b, m, A, B, c, d = tuple([match[i] for i in keys])
if PositiveIntegerQ(m):

This comment has been minimized.

@parsoyaarihant

parsoyaarihant Aug 23, 2017

Contributor

@Abdullahjavednesar , complete these functions

@parsoyaarihant

parsoyaarihant Aug 23, 2017

Contributor

@Abdullahjavednesar , complete these functions

This comment has been minimized.

@Abdullahjavednesar

Abdullahjavednesar Aug 23, 2017

Contributor

@parsoyaarihant, you had implemented ExpandIntegrand, what's incomplete here?

@Abdullahjavednesar

Abdullahjavednesar Aug 23, 2017

Contributor

@parsoyaarihant, you had implemented ExpandIntegrand, what's incomplete here?

This comment has been minimized.

@parsoyaarihant

parsoyaarihant Aug 24, 2017

Contributor

ElementaryFunctionQ, FunctionOfTrigOfLinearQ

@parsoyaarihant

parsoyaarihant Aug 24, 2017

Contributor

ElementaryFunctionQ, FunctionOfTrigOfLinearQ

Abdullahjavednesar and others added some commits Aug 24, 2017

Added documentation in __init__.py file
Added documentation on how to use rubi module and TODO list.

commented out some rules in rubi.py since loading all the rules takes too much time for travis.
@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Aug 31, 2017

Member

@Upabjojr, @Abdullahjavednesar what needs to be done to finish this? Let's get it merged before the GSoC is over, i.e. this week?

Member

certik commented Aug 31, 2017

@Upabjojr, @Abdullahjavednesar what needs to be done to finish this? Let's get it merged before the GSoC is over, i.e. this week?

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Aug 31, 2017

Contributor

what needs to be done to finish this?

I guess we just need to add some @XFAIL or skip the tests that are failing. Some problems are in MatchPy and depend on that project.

Contributor

Upabjojr commented Aug 31, 2017

what needs to be done to finish this?

I guess we just need to add some @XFAIL or skip the tests that are failing. Some problems are in MatchPy and depend on that project.

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Aug 31, 2017

Member

@Abdullahjavednesar can you please skip the failing tests? Also, can you list the problems with MatchPy? Let's figure out what should be done.

Member

certik commented Aug 31, 2017

@Abdullahjavednesar can you please skip the failing tests? Also, can you list the problems with MatchPy? Let's figure out what should be done.

@parsoyaarihant

This comment has been minimized.

Show comment
Hide comment
@parsoyaarihant

parsoyaarihant Sep 1, 2017

Contributor

@certik , there is a bug in MatchPy because of which the tests are not passing.

Shall I skip the test anyway for now?

Contributor

parsoyaarihant commented Sep 1, 2017

@certik , there is a bug in MatchPy because of which the tests are not passing.

Shall I skip the test anyway for now?

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 1, 2017

Member

How many tests are we talking about? If it is just a few, then yes. If it is all of them, then let's see how to fix it.

Member

certik commented Sep 1, 2017

How many tests are we talking about? If it is just a few, then yes. If it is all of them, then let's see how to fix it.

@parsoyaarihant

This comment has been minimized.

Show comment
Hide comment
@parsoyaarihant

parsoyaarihant Sep 1, 2017

Contributor

Its only few(around 1-2). I will update this branch with the changes today.

Contributor

parsoyaarihant commented Sep 1, 2017

Its only few(around 1-2). I will update this branch with the changes today.

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Sep 1, 2017

Contributor

Otherwise just put those tests in a separate location. We need to merge this and for that we need the tests to pass.

Contributor

Upabjojr commented Sep 1, 2017

Otherwise just put those tests in a separate location. We need to merge this and for that we need the tests to pass.

@Abdullahjavednesar

This comment has been minimized.

Show comment
Hide comment
@Abdullahjavednesar

Abdullahjavednesar Sep 1, 2017

Contributor

@parsoyaarihant, are you working on the failing tests?
@certik, Arihant has pointed the issue, once it is fixed I think we'll pass all the tests.

Contributor

Abdullahjavednesar commented Sep 1, 2017

@parsoyaarihant, are you working on the failing tests?
@certik, Arihant has pointed the issue, once it is fixed I think we'll pass all the tests.

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 1, 2017

Member
Member

certik commented Sep 1, 2017

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Sep 1, 2017

Contributor

I agree, we haven't included into master any coffee of your work so far, it's time to merge, whatever the state of the code.

Contributor

Upabjojr commented Sep 1, 2017

I agree, we haven't included into master any coffee of your work so far, it's time to merge, whatever the state of the code.

@parsoyaarihant

This comment has been minimized.

Show comment
Hide comment
@parsoyaarihant

parsoyaarihant Sep 1, 2017

Contributor

Okay, I am working on it right now.

Contributor

parsoyaarihant commented Sep 1, 2017

Okay, I am working on it right now.

parsoyaarihant added some commits Sep 1, 2017

Added new rules by adding seperate With() functions
Previously, we were getting recursion errors due to `With`. I have defiend `With`outside the functions. This helps in preventing recursion errors.

Added error functions in matchpy matcher.

Removed additional parameters of gamma.
@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 1, 2017

Member

I ran tests with Python 2.7:

$ bin/test sympy/integrals/
============================= test process starts ==============================
executable:         /home/certik/ext/miniconda2/envs/jup/bin/python  (2.7.12-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
random seed:        84565124
hash randomization: on (PYTHONHASHSEED=1447910273)

sympy/integrals/tests/test_deltafunctions.py[2] ..                          [OK]
sympy/integrals/tests/test_failing_integrals.py[5] wwwww                    [OK]
sympy/integrals/tests/test_heurisch.py[25] .............www.........        [OK]
sympy/integrals/tests/test_integrals.py[107] .......www.........................
........................................................................    [OK]
sympy/integrals/tests/test_intpoly.py[3] ...                                [OK]
sympy/integrals/tests/test_lineintegrals.py[1] .                            [OK]
sympy/integrals/tests/test_manual.py[22] ......................             [OK]
sympy/integrals/tests/test_meijerint.py[25] ......www................       [OK]
sympy/integrals/tests/test_prde.py[15] ...............                      [OK]
sympy/integrals/tests/test_quadrature.py[16] ................               [OK]
sympy/integrals/tests/test_rationaltools.py[8] ........                     [OK]
sympy/integrals/tests/test_rde.py[9] .........                              [OK]
sympy/integrals/tests/test_risch.py[34] ..................................  [OK]
sympy/integrals/tests/test_singularityfunctions.py[1] .                     [OK]
sympy/integrals/tests/test_transforms.py[18] ...wwwww..........             [OK]
sympy/integrals/tests/test_trigonometry.py[4] ....                          [OK]
________________________________ slowest tests _________________________________
test_fourier_transform - Took 10.020 seconds
test_inverse_laplace_transform - Took 11.187 seconds
test_issue_10847 - Took 11.497 seconds
test_messy - Took 12.258 seconds
test_polytope_integrate - Took 16.016 seconds
test_expint - Took 21.027 seconds
========== tests finished: 276 passed, 19 skipped, in 329.11 seconds ===========

and I am now running it with Python 3.6 + matchpy. It is extremely slow....

It's probably due to the following issue:

In [1]: %time from sympy.integrals.rubi.rubi import rubi_integrate
CPU times: user 2min 8s, sys: 160 ms, total: 2min 9s
Wall time: 2min 9s

It takes over 2 minutes to import rubi_integrate. That will have to be fixed.

Otherwise it seems to work. However, it seems to be easily an order of magnitude slower than our current integrator, e.g.:

In [10]: %time rubi_integrate(x**6/(a + b*x)**2, x)
CPU times: user 4.26 s, sys: 0 ns, total: 4.26 s
Wall time: 4.25 s
Out[10]: 
        6           5                   4        3  2    2  3      4     5 
       a         6⋅a ⋅log(a + b⋅x)   5⋅a ⋅x   2⋅a ⋅x    a ⋅x    a⋅x     x  
- ──────────── - ───────────────── + ────── - ─────── + ───── - ──── + ────
   7                      7             6         5        4       3      2
  b ⋅(a + b⋅x)           b             b         b        b     2⋅b    5⋅b 

In [11]: %time integrate(x**6/(a + b*x)**2, x)
CPU times: user 456 ms, sys: 0 ns, total: 456 ms
Wall time: 458 ms
Out[11]: 
        6          5                   4        3  2    2  3      4     5 
       a        6⋅a ⋅log(a + b⋅x)   5⋅a ⋅x   2⋅a ⋅x    a ⋅x    a⋅x     x  
- ─────────── - ───────────────── + ────── - ─────── + ───── - ──── + ────
     7    8              7             6         5        4       3      2
  a⋅b  + b ⋅x           b             b         b        b     2⋅b    5⋅b 

Why is rubi_integrate 10x slower than integrate?

Why does importing rubi_integrate take over 2 minutes?

Member

certik commented Sep 1, 2017

I ran tests with Python 2.7:

$ bin/test sympy/integrals/
============================= test process starts ==============================
executable:         /home/certik/ext/miniconda2/envs/jup/bin/python  (2.7.12-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
random seed:        84565124
hash randomization: on (PYTHONHASHSEED=1447910273)

sympy/integrals/tests/test_deltafunctions.py[2] ..                          [OK]
sympy/integrals/tests/test_failing_integrals.py[5] wwwww                    [OK]
sympy/integrals/tests/test_heurisch.py[25] .............www.........        [OK]
sympy/integrals/tests/test_integrals.py[107] .......www.........................
........................................................................    [OK]
sympy/integrals/tests/test_intpoly.py[3] ...                                [OK]
sympy/integrals/tests/test_lineintegrals.py[1] .                            [OK]
sympy/integrals/tests/test_manual.py[22] ......................             [OK]
sympy/integrals/tests/test_meijerint.py[25] ......www................       [OK]
sympy/integrals/tests/test_prde.py[15] ...............                      [OK]
sympy/integrals/tests/test_quadrature.py[16] ................               [OK]
sympy/integrals/tests/test_rationaltools.py[8] ........                     [OK]
sympy/integrals/tests/test_rde.py[9] .........                              [OK]
sympy/integrals/tests/test_risch.py[34] ..................................  [OK]
sympy/integrals/tests/test_singularityfunctions.py[1] .                     [OK]
sympy/integrals/tests/test_transforms.py[18] ...wwwww..........             [OK]
sympy/integrals/tests/test_trigonometry.py[4] ....                          [OK]
________________________________ slowest tests _________________________________
test_fourier_transform - Took 10.020 seconds
test_inverse_laplace_transform - Took 11.187 seconds
test_issue_10847 - Took 11.497 seconds
test_messy - Took 12.258 seconds
test_polytope_integrate - Took 16.016 seconds
test_expint - Took 21.027 seconds
========== tests finished: 276 passed, 19 skipped, in 329.11 seconds ===========

and I am now running it with Python 3.6 + matchpy. It is extremely slow....

It's probably due to the following issue:

In [1]: %time from sympy.integrals.rubi.rubi import rubi_integrate
CPU times: user 2min 8s, sys: 160 ms, total: 2min 9s
Wall time: 2min 9s

It takes over 2 minutes to import rubi_integrate. That will have to be fixed.

Otherwise it seems to work. However, it seems to be easily an order of magnitude slower than our current integrator, e.g.:

In [10]: %time rubi_integrate(x**6/(a + b*x)**2, x)
CPU times: user 4.26 s, sys: 0 ns, total: 4.26 s
Wall time: 4.25 s
Out[10]: 
        6           5                   4        3  2    2  3      4     5 
       a         6⋅a ⋅log(a + b⋅x)   5⋅a ⋅x   2⋅a ⋅x    a ⋅x    a⋅x     x  
- ──────────── - ───────────────── + ────── - ─────── + ───── - ──── + ────
   7                      7             6         5        4       3      2
  b ⋅(a + b⋅x)           b             b         b        b     2⋅b    5⋅b 

In [11]: %time integrate(x**6/(a + b*x)**2, x)
CPU times: user 456 ms, sys: 0 ns, total: 456 ms
Wall time: 458 ms
Out[11]: 
        6          5                   4        3  2    2  3      4     5 
       a        6⋅a ⋅log(a + b⋅x)   5⋅a ⋅x   2⋅a ⋅x    a ⋅x    a⋅x     x  
- ─────────── - ───────────────── + ────── - ─────── + ───── - ──── + ────
     7    8              7             6         5        4       3      2
  a⋅b  + b ⋅x           b             b         b        b     2⋅b    5⋅b 

Why is rubi_integrate 10x slower than integrate?

Why does importing rubi_integrate take over 2 minutes?

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 1, 2017

Member

So with Python 2.7 (no matchpy), the sympy/integrals tests take under 3 minutes on my machine. With Python 3.6 + matchpy, in over 45 minutes it only got this far:

$ bin/test sympy/integrals/
============================= test process starts ==============================
executable:         /home/certik/ext/miniconda2/envs/matchpy/bin/python  (3.6.2-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
random seed:        729680
hash randomization: on (PYTHONHASHSEED=4108166084)

sympy/integrals/rubi/parsetools/tests/test_parse.py[1] .                    [OK]
sympy/integrals/rubi/tests/test_1_2.py[8] ..f...

Which test on Travis is testing matchpy? It seems it isn't tested, unless I missed it.

@Upabjojr, @parsoyaarihant, @Abdullahjavednesar, @asmeurer any ideas what we can do to move forward?

One idea could be to run just the following test:

$ bin/test sympy/integrals/rubi/tests/test_rubi_integrate.py 
============================= test process starts ==============================
executable:         /home/certik/ext/miniconda2/envs/matchpy/bin/python  (3.6.2-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
random seed:        73963789
hash randomization: on (PYTHONHASHSEED=73469935)

sympy/integrals/rubi/tests/test_rubi_integrate.py[1] .                      [OK]

================= tests finished: 1 passed, in 165.47 seconds ==================

which finishes in under 3 minutes.

Member

certik commented Sep 1, 2017

So with Python 2.7 (no matchpy), the sympy/integrals tests take under 3 minutes on my machine. With Python 3.6 + matchpy, in over 45 minutes it only got this far:

$ bin/test sympy/integrals/
============================= test process starts ==============================
executable:         /home/certik/ext/miniconda2/envs/matchpy/bin/python  (3.6.2-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
random seed:        729680
hash randomization: on (PYTHONHASHSEED=4108166084)

sympy/integrals/rubi/parsetools/tests/test_parse.py[1] .                    [OK]
sympy/integrals/rubi/tests/test_1_2.py[8] ..f...

Which test on Travis is testing matchpy? It seems it isn't tested, unless I missed it.

@Upabjojr, @parsoyaarihant, @Abdullahjavednesar, @asmeurer any ideas what we can do to move forward?

One idea could be to run just the following test:

$ bin/test sympy/integrals/rubi/tests/test_rubi_integrate.py 
============================= test process starts ==============================
executable:         /home/certik/ext/miniconda2/envs/matchpy/bin/python  (3.6.2-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
random seed:        73963789
hash randomization: on (PYTHONHASHSEED=73469935)

sympy/integrals/rubi/tests/test_rubi_integrate.py[1] .                      [OK]

================= tests finished: 1 passed, in 165.47 seconds ==================

which finishes in under 3 minutes.

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Sep 1, 2017

Contributor

It takes over 2 minutes to import rubi_integrate. That will have to be fixed.

We are well aware of this issue. We have discussed with @wheerd about the possibility of a code generator and that's probably the best solution to this problem. Unfortunately the generated code exceeds 20 MB and apparently that's a Python file size limit or something of a problem like that (if I have understood correctly).

We need to wait until the MatchPy developers solve this issue.

The RUBI tests are run on Python 3.6 only, because that's the only version that MatchPy supports.

Contributor

Upabjojr commented Sep 1, 2017

It takes over 2 minutes to import rubi_integrate. That will have to be fixed.

We are well aware of this issue. We have discussed with @wheerd about the possibility of a code generator and that's probably the best solution to this problem. Unfortunately the generated code exceeds 20 MB and apparently that's a Python file size limit or something of a problem like that (if I have understood correctly).

We need to wait until the MatchPy developers solve this issue.

The RUBI tests are run on Python 3.6 only, because that's the only version that MatchPy supports.

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Sep 1, 2017

Contributor

which finishes in under 3 minutes.

I guess tests for versions of Python other than 3.6 only test the utility functions of RUBI.

Contributor

Upabjojr commented Sep 1, 2017

which finishes in under 3 minutes.

I guess tests for versions of Python other than 3.6 only test the utility functions of RUBI.

@Upabjojr Upabjojr merged commit 9607480 into sympy:master Sep 2, 2017

1 check passed

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

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 2, 2017

Member

@Upabjojr I am actually -1 to merge as it is, because Travis does not test this PR at all.... More over, if you actually run the tests under matchpy, then they hang (!): #13239.

But since it's merged, we need to fix this asap.

Member

certik commented Sep 2, 2017

@Upabjojr I am actually -1 to merge as it is, because Travis does not test this PR at all.... More over, if you actually run the tests under matchpy, then they hang (!): #13239.

But since it's merged, we need to fix this asap.

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 2, 2017

Member

I created a meta issue with all the problems that I encountered so far: #13243.

@Abdullahjavednesar, @parsoyaarihant please try to fix at least the testing issues.

Member

certik commented Sep 2, 2017

I created a meta issue with all the problems that I encountered so far: #13243.

@Abdullahjavednesar, @parsoyaarihant please try to fix at least the testing issues.

@asmeurer

This comment has been minimized.

Show comment
Hide comment
@asmeurer

asmeurer Sep 2, 2017

Member

We can revert the merge if necessary (there is a "revert" button above that will do it).

Member

asmeurer commented Sep 2, 2017

We can revert the merge if necessary (there is a "revert" button above that will do it).

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 2, 2017

Member

I don't want to mess up the history even more. I think most people don't have python 3.6 and matchpy installed, and then they will never see the problems. However, @Abdullahjavednesar, @parsoyaarihant, let's try to fix this quickly please.

Member

certik commented Sep 2, 2017

I don't want to mess up the history even more. I think most people don't have python 3.6 and matchpy installed, and then they will never see the problems. However, @Abdullahjavednesar, @parsoyaarihant, let's try to fix this quickly please.

@Upabjojr

This comment has been minimized.

Show comment
Hide comment
@Upabjojr

Upabjojr Sep 2, 2017

Contributor

I am actually -1 to merge as it is, because Travis does not test this PR at all....

Yes, it does: https://travis-ci.org/sympy/sympy/jobs/270968939

You should read the discussion on gitter, we have extensively discussed about the current problems with Python versions, loading time and MatchPy.

Contributor

Upabjojr commented Sep 2, 2017

I am actually -1 to merge as it is, because Travis does not test this PR at all....

Yes, it does: https://travis-ci.org/sympy/sympy/jobs/270968939

You should read the discussion on gitter, we have extensively discussed about the current problems with Python versions, loading time and MatchPy.

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 2, 2017

Member
Member

certik commented Sep 2, 2017

@parsoyaarihant

This comment has been minimized.

Show comment
Hide comment
@parsoyaarihant

parsoyaarihant Sep 2, 2017

Contributor

. It still hangs on my computer, so that's a big problem,

It will be fixed by #13249 along with other bugfixes

Contributor

parsoyaarihant commented Sep 2, 2017

. It still hangs on my computer, so that's a big problem,

It will be fixed by #13249 along with other bugfixes

@certik

This comment has been minimized.

Show comment
Hide comment
@certik

certik Sep 2, 2017

Member

@parsoyaarihant thanks for the work, indeed it did.

I think we are in good shape regarding this PR, it is tested on Travis and the tests pass on my machine reasonably quickly. So all is good.

Thanks @parsoyaarihant, @Abdullahjavednesar and @Upabjojr for your work on this!

Member

certik commented Sep 2, 2017

@parsoyaarihant thanks for the work, indeed it did.

I think we are in good shape regarding this PR, it is tested on Travis and the tests pass on my machine reasonably quickly. So all is good.

Thanks @parsoyaarihant, @Abdullahjavednesar and @Upabjojr for your work on this!

@parsoyaarihant parsoyaarihant deleted the parsoyaarihant:rubi4 branch Sep 4, 2017

@asmeurer asmeurer referenced this pull request Sep 8, 2017

Open

Rubi integrator #12233

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment