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

Bringup of Rubi section 1 Algebraic functions #2

Merged
merged 24 commits into from
Dec 15, 2024

Conversation

aravindh-krishnamoorthy
Copy link
Collaborator

@aravindh-krishnamoorthy aravindh-krishnamoorthy commented Nov 24, 2024

Goal

Implement the first section of Rubi 1 Algebraic functions with minimal (ideally zero) changes to the Rubi rules (for easy future upgrade and to demonstrate top compatibility with Mathematica).

Howto

Commit: 049d301

In[1]:= << Rubi.m
Loading Rubi 4.17.3.0 will take a minute or two. In the future this will take less than a second.
. . .
Out[1]= None

In[2]:= << Test.m
Out[2]= None

In[3]:= TestRubi["SanityCheck.m"]
. . .
Summary of Integration Test Results
25 integration problems
A - 23 optimal antiderivatives
B - 0 valid but suboptimal antiderivatives
C - 0 unnecessarily complex antiderivatives
D - 0 unable to integrate problems
E - 0 integration timeouts
F - 2 invalid antiderivatives

Rubi results 9.3 times size of optimal antiderivatives on average.

Note: Some progress, but still a long way to go.

Full Test Suite Results for 1 Algebraic functions

TBD

Open Items

These items must be fixed in mathics-core before completing this PR.

See #2 (comment) for the list.

Resolved

  1. Pattern matching issues with two or more default values.
    1. Fixed: 1 / Fix issues with pattern matching for Rubi mathics-core#1176.
  2. With returns unevaluated result
    1. With evaluates the expression but returns unevaluated result.
    2. Fixed: Bugfix in With: Return evaluated result mathics-core#1194
  3. Fix ReadList[] when reading comment-only lines.
    1. Fixed: ReadList[] handling Null and Hold[] expressions mathics-core#1203
  4. TraceEvaluation is not reset after hitting the recursion limit.
    1. Fixed: Reset TraceEvaluation[] params on an exception mathics-core#1204
  5. Increment and related functions hit the recursion limit if the input is undefined.
    1. Issue raised: Increment and related functions hit the recursion limit when the input is undefined mathics-core#1205
  6. Condition within scoping blocks for SetDelayed does not bind to lhs
    1. Issue raised: Condition within With and other scoping environments is disregarded mathics-core#1206
  7. DownValues[f]//FullForm different from Mathematica causing issues in FixIntRules[]
    1. Issue raised: Mathics DownValues[f]//FullForm is different from Mathematica's mathics-core#1209
  8. Select[] with empty sets.
    1. Fixed: Mathics3/mathics-core@ed5e212
  9. Support for Refine[]
    1. Issue raised: Support Refine[] mathics-core#1230
  10. Function modification
    1. Issue raised: Rubi code modification: 9.2 / Function #1 #5

@aravindh-krishnamoorthy
Copy link
Collaborator Author

aravindh-krishnamoorthy commented Dec 1, 2024

@rocky Unfortunately, TestRubi["1 Algebraic functions"] doesn't work because the TimeConstrained based on stopit isn't stopping execution...

I think we're ready to execute the whole suit now. Some tests will still fail and need to be fixed. Identifying these failing tests will aid with the fix.

@rocky
Copy link
Member

rocky commented Dec 1, 2024

@rocky Unfortunately, TestRubi["1 Algebraic functions"] doesn't work because the TimeConstrained based on stopit isn't stopping execution...

I think we will be able to fix or address this, but I would like more detail and clarification about this comment. I don't fully understand what you mean.

I just spent some time running TestRubi. Running things is slow and after about 40 minutes I gave up, after getting to problem 84 of 1917 problems. Attached is a log of my run.

TestRubi.log

In the log I see timeout exceptions like:

Problem 39:
Hold[IntegrationTestProgram`Private`Int[Sqrt[c + d (a + b x)], x]]
Exception ignored in: <function WeakSet.__init__.<locals>._remove at 0x7f4bc0d1b600>
Traceback (most recent call last):
  File "/tmp/.pyenv/versions/3.12.7/lib/python3.12/_weakrefset.py", line 39, in _remove
    def _remove(item, selfref=ref(self)):

stopit.utils.TimeoutException:
Result optimal and 2 fewer steps used.

It is not clear to me though that this is a stopit problem as opposed to a general problem with how Python works.

In Python, an exception can occur at a place where Python is not in a position to be able to handle it. This is called an "unraisable exception". See the second paragraph in https://vstinner.github.io/sys-unraisablehook-python38.html:

An "unraisable exception" is an error which happens when Python cannot report it to the caller. Examples: object finalizer error (del()), weak reference callback failure, error during a GC collection. At the C level, the PyErr_WriteUnraisable() function is called to handle such exception.

And this seems to be what appears here. Notice the mention of deleting a weak reference.

I think we're ready to execute the whole suit now.

If this is going to take days, then why do this instead of running on some small set, fix those problems, and then iterate?
So I understand, what was keeping us from executing the whole test suite before? Was it just coding around some Mathics3 test framework code, like Test.m?

Some tests will still fail and need to be fixed. Identifying these failing tests will aid with the fix.

I have been following along what has been going on and I am very impressed and thankful for this work.

Very few times do we have people using Mathics3 programs to get large projects done. But the best way for the code to improve is for us to do more activity like this. I have tried to write WMA/Mathics3 code, but my WMA skills are seriously lacking.

I see that you've been working around existing bugs, such as for ReadList, or Select. To the extent this helps progress, this is good. And it is possible that we could put the workaround code into Mathics-core.

What I'd ask though is to create bug tickets for each bug you have worked around. And it would be good to keep track of what should happen or be undone when a bug is fixed in Mathics core.

If something is seriously blocking progress on Rubi, then mark it as such. If there is a workaround, the issue has a section to describe that. We should try to prioritize the bugs since there are likely to be many of these.

One other thing, we should consider merging the branch into master. Having branches with lots of commits is not a good idea unless this is a major refactor of existing working code. Here, there was no existing working code.

Again thanks for undertaking this.

@aravindh-krishnamoorthy
Copy link
Collaborator Author

Thank you very much for the kind words and the details on the inner workings of Python. Below, I'll try to explain the issue with TimeConstrained in detail.

@rocky Unfortunately, TestRubi["1 Algebraic functions"] doesn't work because the TimeConstrained based on stopit isn't stopping execution...

I think we will be able to fix or address this, but I would like more detail and clarification about this comment. I don't fully understand what you mean.

On my PC, the following does not abort and return after $30$ seconds. Instead, Int executes all the way to the end.

In[1]:= << Rubi.m
In[2]:= << Test.m
In[3]:= TimeConstrained[Int[(a + b*x)^5/x^1, x], 30, 0]

However, smaller timeout values such as $2$ seconds work as expected, i.e., with a timeout of $2$ seconds, the evaluation aborts after $2$ seconds and $0$ is returned. It works until about $15$ seconds as expected. But beyond this, e.g., $30$ seconds, the evaluation does not time out. On your PC, which is presumably faster, I'd suspect that you'll have to modify the timeout to something other than $30$ seconds to observe this issue.

@rocky
Copy link
Member

rocky commented Dec 2, 2024

Thank you very much for the kind words and the details on the inner workings of Python. Below, I'll try to explain the issue with TimeConstrained in detail.

@rocky Unfortunately, TestRubi["1 Algebraic functions"] doesn't work because the TimeConstrained based on stopit isn't stopping execution...

I think we will be able to fix or address this, but I would like more detail and clarification about this comment. I don't fully understand what you mean.

On my PC, the following does not abort and return after 30 seconds. Instead, Int executes all the way to the end.

In[1]:= << Rubi.m
In[2]:= << Test.m
In[3]:= TimeConstrained[Int[(a + b*x)^5/x^1, x], 30, 0]

However, smaller timeout values such as 2 seconds work as expected, i.e., with a timeout of 2 seconds, the evaluation aborts after 2 seconds and 0 is returned. It works until about 15 seconds as expected. But beyond this, e.g., 30 seconds, the evaluation does not time out. On your PC, which is presumably faster, I'd suspect that you'll have to modify the timeout to something other than 30 seconds to observe this issue.

Ok - thanks for the more detailed explanation. I will look at this when I am able to, and let you know what we might be able to do.

@rocky
Copy link
Member

rocky commented Dec 2, 2024

I haven't started tracking down what's up with TimeConstrained timeout signals yet. However, in preparation for this, I've modified the Mathics3 debugger (now called mathics3-trepan) to be able to trap signals.

I used this to find out what's happening with the delay in FixIntRules[] on line 184.

Not surprisingly , I am seeing a lot of time in the pattern-matching part of Mathics3, specifically, in walk_levels.

FixInt-delays

Speeding up pattern matching is one of the 6 or so big things we need to correct in Mathics3.

@rocky
Copy link
Member

rocky commented Dec 2, 2024

In[1]:= << Rubi.m
In[2]:= << Test.m
In[3]:= TimeConstrained[Int[(a + b*x)^5/x^1, x], 30, 0]

Here is what's happening. While in the middle of one TimeConstrained[] another is getting invoked. This is causing updating the stop timer for later. And then before either completes, another TimeConstrained[] gets called. And so on...

With short timeouts, the chance of hitting another TimeConstrained[] is reduced.

One could conceive of keeping track of all the pending time-constrained evaluations along with their due dates, adjusting deadline times, as new requests that come in based on the ones that already exist. But that is a bit more than I think we want to undertake right now .

Branch at-most-one-TimeConstrained has code that I think you can use.

I believe Rubi code embraces the idea that TimeConstrained[] evaluation is nice to have, but not strictly necessary. In other words, any evaluation that wraps TimeConstrained is allowed to fail, which we will do on those requests after the first one.

The whole TimeConstrained notion is probably the main source of Rubi indeterminacy of solutions - on faster computers, more searching is done.

@aravindh-krishnamoorthy
Copy link
Collaborator Author

aravindh-krishnamoorthy commented Dec 2, 2024

Branch at-most-one-TimeConstrained has code that I think you can use.

Perfect! Thank you, I'll try the branch now and merge this PR as soon as I have the first full run (along with raising other issues, etc.)

Speeding up pattern matching is one of the 6 or so big things we need to correct in Mathics3.

I fully agree. After Rubi, I'd like to implement missing linear algebra functions (my field) and also work on the half-done Jupyter plugin. Soon after, I'll take a look at pattern matching again. Eventually, I want to implement Matrix Calculus on Mathics. It's a grand plan and I'll also be a bit constrained on time as soon as I begin my new job. But, I'll keep contributing!

@aravindh-krishnamoorthy
Copy link
Collaborator Author

aravindh-krishnamoorthy commented Dec 3, 2024

Open Items

Rubi code modified (!)

Enable Steps

Seriously, why do I hate steps that much? To be enabled before completing this PR.
(Because of this: Mathics3/mathics-core#1209)

Support ReplacePart with three arguments

This is nonstandard according to ReplacePart wolfram language documentation. Hence, this will not be fixed.

Support Refine

Issue raised: Mathics3/mathics-core#1230

ReadList

The behaviour of `ReadList` needs attention. Presently, in Mathematica
>> ReadList[StringToStream["(**)\n\n\n{0, x, 1, 0}\n{1, x, 1, x}"], Expression]
{Null,{0,x,1,0},{1,x,1,x}} 

and in Mathics

>> ReadList[StringToStream["(**)\n\n\n{0, x, 1, 0}\n{1, x, 1, x}"], Expression]
{{},{0,x,1,0},{1,x,1,x}}

The current tests are updated to Mathics. This must be checked once the bring up is nearing completion.

With and other scoping environments

Fix the hack in

Mathics3-Rubi/Rubi.m

Lines 195 to 198 in f6b8f7a

Unprotect[With];
With[vars_, Verbatim[Condition][expr_, wcond_]] := Condition[With[vars, expr], With[vars, wcond]];
With[vars_, Verbatim[Condition][expr_, wcond_]] := Condition[Indeterminate, Not[With[vars, wcond]]];
Protect[With];
in pattern.py. Without the fix, Inderminate results are returned.

Increment and friends

Infinite recursion when the input is undefined.

TraceEvaluation after hitting $RecusionLimit

`TraceEvaluation` is not reset after hitting the recursion limit.
In[1]:= f[x_] := x + f[x-1]
Out[1]= None
In[2]:= f[0] = 0
Out[2]= 0
In[3]:= f[201] // TraceEvaluation
  Evaluating: Global`f[201]
    Evaluating: Global`f
. . .
$RecursionLimit::reclim: Recursion depth of 200 exceeded.
Out[3]= $Aborted
In[7]:= 1+2
Evaluating: System`Plus[1, 2]
  Evaluating: System`Plus
Out[7]= 3

Tracing continues for 1+2.

TraceEvaluation not printing the first level

TraceEvaluation does not print the first level if reevaluate is False.
(Note: not a good idea. Hence, do not fix.)

Select with empty sets and three arguments

Fix: Mathics3/mathics-core@ed5e212

rocky added a commit to Mathics3/mathics-core that referenced this pull request Dec 4, 2024
This should address the problem seen via Rubi testing where TraceEvalution was called and we hit a recursion limit.

See Mathics3/Mathics3-Rubi#2 (comment)

---------

Co-authored-by: Aravindh Krishnamoorthy <[email protected]>
rocky added a commit to Mathics3/mathics-core that referenced this pull request Dec 4, 2024
See the `ReadList[]` example in Mathics3/Mathics3-Rubi#2

---------

Co-authored-by: Aravindh Krishnamoorthy <[email protected]>
@aravindh-krishnamoorthy
Copy link
Collaborator Author

I think this will be ready and merged in a day or two.

@rocky
Copy link
Member

rocky commented Dec 9, 2024

@aravindh-krishnamoorthy Note that some misfeatures around TraceEvaluation, Select, ReadList, and Increment/Decrement have been addressed. So if those workarounds are removed and Rubi is less modified, that would be good.

If there is a Mathics3 code-based workaround for Mathics3/mathics-core#1209 that would be nice to add. But see also
Mathics3/mathics-core#1213

Finally, it would be nice to have issues logged in mathics core for the empty sets with 3 args, the specific kinds of problems with TimedConstrained and ReplaceParts with 3 args. In the issue please add reproducible test cases that demonstrate the specific kinds problems you encountered and can be used to demonstrate that those problems were fixed.

Thanks for doing this and for working on Rubi.

@aravindh-krishnamoorthy
Copy link
Collaborator Author

@aravindh-krishnamoorthy Note that some misfeatures around TraceEvaluation, Select, ReadList, and Increment/Decrement have been addressed. So if those workarounds are removed and Rubi is less modified, that would be good.

If there is a Mathics3 code-based workaround for Mathics3/mathics-core#1209 that would be nice to add. But see also Mathics3/mathics-core#1213

Finally, it would be nice to have issues logged in mathics core for the empty sets with 3 args, the specific kinds of problems with TimedConstrained and ReplaceParts with 3 args. In the issue please add reproducible test cases that demonstrate the specific kinds problems you encountered and can be used to demonstrate that those problems were fixed.

Definitely! I will complete the items on the checklist before finalising this. I'll also have to update the README.md file. Then, I'll start working on the mathics-core fixes for the issues I've raised.

rocky added a commit to Mathics3/mathics-core that referenced this pull request Dec 11, 2024
* Add 3-argument Select, by adding a parameter inside Structure's filter function.
* Go over Structure class: remove duplication of filter() and add the count functionality needed by Select[]
* Update Select doctests for 3 arg form. Move error checking to pytest where it belongs.

`Select[]` count-parameter issue is mentioned in Mathics3/Mathics3-Rubi#2
@aravindh-krishnamoorthy aravindh-krishnamoorthy marked this pull request as ready for review December 15, 2024 18:30
@aravindh-krishnamoorthy aravindh-krishnamoorthy merged commit 742eb7e into master Dec 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants