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

Rubi progress and tests #14756

Merged
merged 51 commits into from
Jul 28, 2018
Merged

Rubi progress and tests #14756

merged 51 commits into from
Jul 28, 2018

Conversation

ashishkg0022
Copy link
Contributor

@ashishkg0022 ashishkg0022 commented May 30, 2018

  • integrals
    • Made rubi stable.
    • Added missing utility functions
    • Added large test set

asmeurer and others added 12 commits May 18, 2018 00:45
I pulled out the common functionality to be evaluated once, and used array()
instead of asarray() (they are equivalent, but array() is faster).

This is a partial fix to sympy#14671. It isn't as fast as it used to be yet. I
don't know how to make the decorator faster, other than code generating the
array wrapping directly, which would require improvements to lambdify to work
(lambdify would need to be able to code generation full Python functions).
@ashishkg0022
Copy link
Contributor Author

In original test suite for trinomial products there are 5 files. In sympy format I have kept it here. https://github.com/ashishkg0022/rubi-structure/tree/master/test/trinomial . This contains all trinomial tests from the rubi test-suite.
I have added test_trinomial_passed in this PR . This contains tests which are passed for trinomial_1 . Almost 600 test have passed for trinomial_1 . I started testing trinomial_2 . Then I encountered with issues like

  • With Function is not handled properly. It should be parsed correctly.

  • Some times Matchpy wrongly matches x with some integer

  • and some small issues.
    The most important for now is fixing the With issue. Then majority of trinomial_2 should pass.

@ashishkg0022
Copy link
Contributor Author

ping @Upabjojr @parsoyaarihant

@ashishkg0022 ashishkg0022 changed the title Rubi progress and tests [Not for merging]Rubi progress and tests May 30, 2018
@ashishkg0022
Copy link
Contributor Author

Also

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

In [2]: b*e - c*d
Out[2]: b*e - c*d

but in mathematica

In[1]:= b*e - c*d
Out[1]= -cd+be

This results in difference because there is a function PosQ which treats b*e -c*d True and -c*d +b*e False

@arihantparsoya
Copy link
Contributor

arihantparsoya commented May 31, 2018

Some times Matchpy wrongly matches x with some integer

This should be reported to MatchPy developers. Can you share a sample case where this fails?

@arihantparsoya
Copy link
Contributor

arihantparsoya commented May 31, 2018

Regadring difference in output of SymPy and Mathematica, Mathematica sorts the expression canonically. See: http://reference.wolfram.com/language/ref/Orderless.html http://reference.wolfram.com/language/ref/Sort.html

You can use Sort in the utility function to address this issue.

@ashishkg0022
Copy link
Contributor Author

You can use Sort in the utility function to address this issue.

It handles rest of the cases . But specifically the case b*e -c*d is not sorted by the Sort function too.

@arihantparsoya
Copy link
Contributor

You can create this as a SymPy issue. We might have to improve the Sort function we already have to take care of this.

@Abdullahjavednesar
Copy link
Member

@ashishkg0022 for those utility functions which you've recently modified have you added relevant tests for all (which probably was failing before)? Although I've seen your work before looks good to me, but if you are missing anywhere just recheck.

@ashishkg0022
Copy link
Contributor Author

Yes, I think I have added test cases for them.

@ashishkg0022
Copy link
Contributor Author

There are some more things that need to be done to improve utility functions.
I have opened a separate PR especially for improving utility functions. ( #14956)

@ashishkg0022
Copy link
Contributor Author

I apologise, I have not run pyflakes on it. I was not aware of it (pyflakes). I will run it today and fixed if something is undefined. I will fix it today. Then this can be merged.

@sympy-bot
Copy link

sympy-bot commented Jul 24, 2018

Hi, I am the SymPy bot (v119). I'm here to make sure this pull request has a release notes entry. Please read the guide on how to write release notes.

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

<!-- BEGIN RELEASE NOTES -->
* integrals
    * Made rubi stable.
    * Added missing utility functions
    * Added large test set

<!-- END RELEASE NOTES -->

Your release notes are in good order.

Here is what the release notes will look like:

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

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.

Update

The release notes on the wiki have been updated.

@ashishkg0022
Copy link
Contributor Author

Fixed few undefined error using pyflakes.

@certik
Copy link
Member

certik commented Jul 25, 2018

@Upabjojr would you mind reviewing this PR please and give it a final +1?

@Upabjojr
Copy link
Contributor

@ashishkg0022 did you add the instructions on how to run the tests?

Parsing Rules and Tests
=======================
Code for parsing rule and tests are included in sympy.
They have been properly explained with steps in `rubi_parsing_guide.md` in parsetools.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

full path of rubi_parsing_guide.md is better

@@ -671,6 +671,7 @@ def _doctest(*paths, **kwargs):
"sympy/physics/unitsystems.py", # raises deprecation warning
"sympy/parsing/latex/_antlr/latexlexer.py", # generated code
"sympy/parsing/latex/_antlr/latexparser.py", # generated code
"sympy/integrals/rubi/rubi.py"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we blacklisting the doctests as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only for rubi.py, not for utility_functions.
Actually, the loading time is around 10 - 11 minutes. So, travis fails. If nothing happens in 10 minutes, travis fails. So I have blacklisted it.

@Abdullahjavednesar Abdullahjavednesar added the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Jul 25, 2018
@Upabjojr
Copy link
Contributor

I guess the master branch should be merged into this PR.

@ashishkg0022
Copy link
Contributor Author

I guess the master branch should be merged into this PR.

I have merged the master branch

@ashishkg0022
Copy link
Contributor Author

The travis failed due to some network error.

@Upabjojr
Copy link
Contributor

Some of the rubi tests keep failing. For example, test_1_3.py, is it supposed to be like that?

@ashishkg0022
Copy link
Contributor Author

test_1_3.py is from last year. I have not run it due to a large time taken by it.
What is the status of newly added test files? Do a lot of them fail?

@Upabjojr
Copy link
Contributor

Do a lot of them fail?

I didn't test them all so far.

@ashishkg0022
Copy link
Contributor Author

Can you please run them all? I have tested them in part. I have not run them all together. Like, when I tested exponentials they passed. But after a week I made some changes in trigo rules and utility functions. So it might be possible very few might fail because of some changes done later. For any new change, it's not possible to run all tests again.

@asmeurer
Copy link
Member

Regarding the release notes, I accidentally added rubi as a submodule, but since it's been moved to integrals.rubi, the header should just be "integrals" (I've fixed it). c.f. sympy/sympy-bot#16. If there end up being a lot of integrals release notes we can split them up into different subsubmodules like integrals.risch, integrals.rubi, and so on.

@Upabjojr
Copy link
Contributor

@ashishkg0022
Copy link
Contributor Author

I saw it. Tests which I have run recently like hyperbolic are passing but a few of trinomials and miscellaneous algebra might be failing. Is there some way I can get a list of all expressions in the test set which are failing?

@Upabjojr
Copy link
Contributor

You have the line numbers. Some tests are actually still running.

@Upabjojr
Copy link
Contributor

Shall we merge this PR?

I have still two tests of rubi_tests running, so far it's 30 hours. I will update the output once they are all finished.

@Upabjojr
Copy link
Contributor

@parsoyaarihant @Abdullahjavednesar ok with merging?

@Upabjojr Upabjojr merged commit bccd4f3 into sympy:master Jul 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrals.rubi PR: author's turn The PR has been reviewed and the author needs to submit more changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants