✅

Hi, I am the SymPy bot (v167). 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:

• integrals
  • Fix integrate(1/sqrt(x**2-1)) (#23573 by @eagleoflqj)

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

<!-- 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://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->

#23566 heurisch part. It shouldn't be closed.
#### Brief description of what is fixed or changed

Use `log` instead of `acosh`. ~~Implement a special case for evaluating derivative of log.~~ Patch heurisch to simplify a special case of derivative. 
#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

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 -->
* integrals
  * Fix integrate(1/sqrt(x**2-1))
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

With checking s**2 and S.Half, any expressions without these 2 elements won't be affected much. With multi-stage match, most irrelevant expressions will fail and break early. The expressions that pass all matches are worth a try to simplify.

I'll repeat what I said earlier:

Unless there is a bug in the derivative of log I would rather just XFAIL the integrals test than make any changes to the derivative of log regardless of speed.

While the speed does concern me the more important point is that the code for differentiation of logs should not be complicated by adding checks for random special cases. Just think for a moment how many other possible expression patterns we could check for here: this is clearly not a scalable way to implement differentiation. Ideally we would use much better algorithms for differentiation but that's impossible if those algorithms are expected to produce arbitrarily defined results in random special cases.

It’s not a random special case. It helps fixing a long standing bug elsewhere, meanwhile provides a nicer result for derivative itself. SymPy is a rule-based library so there will never be too many rules to add (think about the model that SymPy tries to compete: mathematica, its size is perhaps 100x of SymPy, it even hard-codes solution for some special quintic equations). There are only awkward rules that should be replaced by smarter rules.

It helps fixing a long standing bug elsewhere,

That bug can be fixed without the change here. If necessary the integral test can be XFAILed. It previously succeeded only by depending on buggy behaviour in the derivative of acosh and the result that it tested was incorrect.

Fixing acosh.diff now prevents heurisch from giving a wrong result in this case which is good. Of course it is good to investigate the problem with heurisch and to think about other improvements that are related to the change (as you are doing). Finding some other way for heurisch to evaluate that integral correctly is not necessarily a precondition for fixing the bug in acosh.diff though.

Making this particular integral work is not a reason to change log.diff. The complicated derivative that you showed should be easier to simplify but that's a bug in the simplification code. The output from log is perfectly correct and fine and follows the obvious definition for differentiation of logs:

In [8]: a, b, c = symbols('a, b, c')

In [9]: F = log(b + 2*sqrt(c)*sqrt(a + b*x + c*x**2) + 2*c*x)

In [10]: F.diff(x)
Out[10]: 
             ⎛b      ⎞              
        2⋅√c⋅⎜─ + c⋅x⎟              
             ⎝2      ⎠              
     ─────────────────── + 2⋅c      
        ________________            
       ╱              2             
     ╲╱  a + b⋅x + c⋅x              
────────────────────────────────────
            ________________        
           ╱              2         
b + 2⋅√c⋅╲╱  a + b⋅x + c⋅x   + 2⋅c⋅x

That's a nice clear application of the chain rule for the log and then the sqrt which is precisely what it should be. The problem of course is why it doesn't simplify e.g.:

In [7]: F.diff(x).factor()
Out[7]: 
                                 ________________         
                  3/2           ╱              2          
        b⋅√c + 2⋅c   ⋅x + 2⋅c⋅╲╱  a + b⋅x + c⋅x           
──────────────────────────────────────────────────────────
   ________________ ⎛            ________________        ⎞
  ╱              2  ⎜           ╱              2         ⎟
╲╱  a + b⋅x + c⋅x  ⋅⎝b + 2⋅√c⋅╲╱  a + b⋅x + c⋅x   + 2⋅c⋅x⎠

The reason is that most simplification routines depend in some way on the polynomial code but the polynomial code doesn't understand how to treat sqrt(c) and c as being related e.g.:

In [3]: Poly(sqrt(c) + c**2)
Out[3]: Poly(c**2 + (sqrt(c)), c, sqrt(c), domain='ZZ')

In [4]: Poly(sqrt(c) + c**2).gens
Out[4]: (c, √c)

In [5]: Poly(sqrt(c) + c**2) ** 2
Out[5]: Poly(c**4 + 2*c**2*(sqrt(c)) + (sqrt(c))**2, c, sqrt(c), domain='ZZ')

Notice that c and sqrt(c) are being treated as independent symbols (generators). That means that polys doesn't know that sqrt(c)**2 = c and it also doesn't understand that c can be factored as sqrt(c)*sqrt(c). This causes all sorts of things cancel, factor, together etc to fail.

Something like this can make the polys code smarter about square-rooted symbols (although it needs to be applied in more places):

diff --git a/sympy/polys/polyutils.py b/sympy/polys/polyutils.py
index 82c8c83..1798171 100644
--- a/sympy/polys/polyutils.py
+++ b/sympy/polys/polyutils.py
@@ -273,6 +273,28 @@ def _is_coeff(factor):
 
         reprs.append(terms)
 
+    #
+    # See if e.g. sqrt(c) and c are both in gens
+    # If they are then rewrite c as sqrt(c)**2
+    #
+    to_replace = {}
+    for g1 in gens:
+        if g1.is_Pow and g1.base in gens and g1.exp.is_Rational and g1.exp.p == 1:
+            to_replace[g1.base] = (g1, g1.exp.q)
+
+    if to_replace:
+
+        to_replace_set = to_replace.keys()
+
+        for terms in reprs:
+            for coeff, term in terms:
+                for g2 in to_replace_set & term:
+                    exp2 = term.pop(g2)
+                    g1, exp1 = to_replace[g2]
+                    term[g1] = term.get(g1, 0) + exp1*exp2
+
+        gens -= to_replace_set
+
     gens = _sort_gens(gens, opt=opt)
     k, indices = len(gens), {}
 

With that you can have:

In [13]: F.diff(x).factor()
Out[13]: 
         √c        
───────────────────
   ________________
  ╱              2 
╲╱  a + b⋅x + c⋅x  

the polynomial code doesn't understand how to treat sqrt(c) and c as being related e.g.:

Yes. I have a local fix for polynomial but it breaks some solver tests and I haven't fixed them.
OK, I'll try to patch heurisch code.

How to XFAIL this test? I'm not familiar with ODE.

You can do it like

How to XFAIL this test?

Why does it fail?

I've mentioned it before, but it would really help to have a separate method for computing derivatives just for heurisch. See #4300 and the cross-linked issues.

Why does it fail?

The 2 new solutions are valid only on a given range related to C1:

from sympy import *

x,C1 = symbols('x C1')

def test(f):
    equ = f**2 + (x*sqrt(f**2 - x**2) - x*f)*f.diff(x)
    print(equ.subs({x: -1, C1: -1}).n())
    print(equ.subs({x: 1, C1: -1}).n())
    print(equ.subs({x: -1, C1: 1}).n())
    print(equ.subs({x: 1, C1: 1}).n())

test(-sqrt(-x*exp(2*C1)/(x - 2*exp(C1))))
test(sqrt(-x*exp(2*C1)/(x - 2*exp(C1))))

output:

0.e-126
-2.45041787584235
0.e-125
0.e-125
-0.122888165622335
0.e-125
-1.32633468246037
1.29008974262908

I XFAIL it for checksol.

I've mentioned it before, but it would really help to have a separate method for computing derivatives just for heurisch.

I saw PRs aimed for it. They are not finished as the problem is not as trivial as previously thought. To fix #23566 that seems over complicated.

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

       before           after         ratio
     [77f1d79c]       [d142bcfd]
     <sympy-1.10.1^0>                 
+         129±3ms         225±10ms     1.75  sum.TimeSum.time_doit

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

I XFAIL it for checksol.

Would the previous solution also fail if the derivative of acosh was fixed as in #23566?

Would the previous solution also fail if the derivative of acosh was fixed as in #23566?

No, to my surprise it still passes.
It is because when checking solution, the solution is posifyed at

So whether acosh(x).diff(x)==1/sqrt(x**2-1) or acosh(x).diff(x)==1/(sqrt(x+1)*sqrt(x-1)) doesn't matter here:

x,y=symbols('x y', positive=True)
simplify(sqrt(x+y)*sqrt(x-y))  # == sqrt(x**2-y**2)

the solution is posifyed at

Yeah, that shouldn't be there...

I think this looks good.