[GSoC 1.1]Fixes for _eval_nseries of Abs class

[arnabnandikgp]

References to other Issues or PRs

Fixes #25682
Fixes #25681
Closes #25756

Brief description of what is fixed or changed

Other comments

comment#25682

Release Notes

• functions
  • Fixed bugs in _eval_nseries method of Abs class

[sympy-bot]

✅

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

• functions
  • Fixed bugs in _eval_nseries method of Abs class (#26349 by @arnabnandikgp)

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

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

<!-- 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
Fixes #25682 
Fixes #25681 
Closes #25756
<!-- 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. -->


#### Brief description of what is fixed or changed


#### Other comments
[comment#25682](https://github.com/sympy/sympy/issues/25682#issuecomment-1988436674)

#### 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. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

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 -->
* functions
  * Fixed bugs in `_eval_nseries` method of Abs class 

<!-- END RELEASE NOTES -->

[arnabnandikgp]

ping @anutosh491 for reviews.

[github-actions]

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)

| Change   | Before [a00718ba]    | After [226096b8]    |   Ratio | Benchmark (Parameter)                                                |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| -        | 67.4±0.6ms           | 44.0±0.3ms          |    0.65 | integrate.TimeIntegrationRisch02.time_doit(10)                       |
| -        | 67.0±0.9ms           | 42.9±0.1ms          |    0.64 | integrate.TimeIntegrationRisch02.time_doit_risch(10)                 |
| +        | 18.6±0.09μs          | 29.6±0.3μs          |    1.59 | integrate.TimeIntegrationRisch03.time_doit(1)                        |
| -        | 5.35±0.06ms          | 2.84±0.01ms         |    0.53 | logic.LogicSuite.time_load_file                                      |
| -        | 73.1±0.4ms           | 28.5±0.07ms         |    0.39 | polys.TimeGCD_GaussInt.time_op(1, 'dense')                           |
| -        | 25.4±0.07ms          | 16.7±0.07ms         |    0.66 | polys.TimeGCD_GaussInt.time_op(1, 'expr')                            |
| -        | 72.7±0.6ms           | 28.6±0.3ms          |    0.39 | polys.TimeGCD_GaussInt.time_op(1, 'sparse')                          |
| -        | 252±1ms              | 124±0.8ms           |    0.49 | polys.TimeGCD_GaussInt.time_op(2, 'dense')                           |
| -        | 252±2ms              | 125±0.9ms           |    0.49 | polys.TimeGCD_GaussInt.time_op(2, 'sparse')                          |
| -        | 649±3ms              | 371±2ms             |    0.57 | polys.TimeGCD_GaussInt.time_op(3, 'dense')                           |
| -        | 655±3ms              | 369±2ms             |    0.56 | polys.TimeGCD_GaussInt.time_op(3, 'sparse')                          |
| -        | 494±1μs              | 284±2μs             |    0.57 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense')            |
| -        | 1.81±0.01ms          | 1.07±0ms            |    0.59 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense')            |
| -        | 5.77±0.02ms          | 3.06±0.02ms         |    0.53 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense')            |
| -        | 446±1μs              | 230±2μs             |    0.52 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense')               |
| -        | 1.45±0.01ms          | 678±7μs             |    0.47 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense')               |
| -        | 4.80±0.04ms          | 1.65±0.01ms         |    0.34 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense')               |
| -        | 372±2μs              | 209±2μs             |    0.56 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense')                |
| -        | 2.41±0.02ms          | 1.22±0.01ms         |    0.5  | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense')                |
| -        | 10.1±0.07ms          | 4.36±0.01ms         |    0.43 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense')                |
| -        | 357±2μs              | 168±0.6μs           |    0.47 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense')            |
| -        | 2.46±0.02ms          | 899±9μs             |    0.37 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense')            |
| -        | 9.63±0.08ms          | 2.69±0.01ms         |    0.28 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense')            |
| -        | 1.03±0.01ms          | 432±3μs             |    0.42 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense')           |
| -        | 1.73±0.1ms           | 502±0.8μs           |    0.29 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse')          |
| -        | 5.91±0.03ms          | 1.79±0.02ms         |    0.3  | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense')           |
| -        | 8.47±0.1ms           | 1.49±0.01ms         |    0.18 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse')          |
| -        | 285±0.5μs            | 64.2±0.3μs          |    0.23 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse')             |
| -        | 3.39±0.04ms          | 396±3μs             |    0.12 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense')              |
| -        | 3.98±0.03ms          | 280±1μs             |    0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse')             |
| -        | 6.96±0.05ms          | 1.28±0ms            |    0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense')              |
| -        | 8.70±0.06ms          | 824±2μs             |    0.09 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse')             |
| -        | 5.06±0.02ms          | 2.98±0.01ms         |    0.59 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| -        | 12.2±0.06ms          | 6.60±0.02ms         |    0.54 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense')  |
| -        | 22.3±0.05ms          | 8.98±0.03ms         |    0.4  | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| -        | 5.24±0.03ms          | 864±3μs             |    0.16 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse')    |
| -        | 12.6±0.09ms          | 7.05±0.02ms         |    0.56 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse')    |
| -        | 102±0.6ms            | 25.9±0.1ms          |    0.25 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense')     |
| -        | 166±0.8ms            | 53.5±0.2ms          |    0.32 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse')    |
| -        | 173±2μs              | 113±0.7μs           |    0.65 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense')      |
| -        | 362±2μs              | 216±0.8μs           |    0.6  | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse')     |
| -        | 4.25±0.02ms          | 842±4μs             |    0.2  | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense')      |
| -        | 5.30±0.04ms          | 382±2μs             |    0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse')     |
| -        | 20.2±0.2ms           | 2.80±0.01ms         |    0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense')      |
| -        | 22.9±0.2ms           | 625±2μs             |    0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse')     |
| -        | 478±2μs              | 138±1μs             |    0.29 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| -        | 4.65±0.02ms          | 625±4μs             |    0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense')  |
| -        | 5.23±0.03ms          | 140±3μs             |    0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| -        | 13.1±0.07ms          | 1.28±0.01ms         |    0.1  | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense')  |
| -        | 13.8±0.1ms           | 142±0.5μs           |    0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| -        | 133±0.8μs            | 74.3±1μs            |    0.56 | solve.TimeMatrixOperations.time_rref(3, 0)                           |
| -        | 251±0.8μs            | 87.5±0.4μs          |    0.35 | solve.TimeMatrixOperations.time_rref(4, 0)                           |
| -        | 23.9±0.05ms          | 10.2±0.05ms         |    0.43 | solve.TimeSolveLinSys189x49.time_solve_lin_sys                       |
| -        | 28.2±0.2ms           | 15.5±0.1ms          |    0.55 | solve.TimeSparseSystem.time_linsolve_Aaug(20)                        |
| -        | 54.6±0.6ms           | 24.9±0.09ms         |    0.46 | solve.TimeSparseSystem.time_linsolve_Aaug(30)                        |
| -        | 28.1±0.3ms           | 15.2±0.05ms         |    0.54 | solve.TimeSparseSystem.time_linsolve_Ab(20)                          |
| -        | 53.9±0.2ms           | 24.7±0.1ms          |    0.46 | solve.TimeSparseSystem.time_linsolve_Ab(30)                          |

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

[smichr]

Have you considered the comment here in your work?

[arnabnandikgp]

Thanks for looking into this, I was thinking about commenting my thoughts about the reference comment here for some time now, so basically the referenced comment states that for something like abs(expr).series(x) we can start out by first calculating the series expansion of the expr i.e expr.series(x) upto the first non constant term, so for something like this a + b*x**e ,we have the corresponding terms of abs(expr).series(x) as Abs(a) + re(b/sign(a)*cdir**e)*cdir**-e*x**e which in light of a real cdir will be simplified to Abs(a) + re(b/sign(a))*x**e in its most general form. Now taking these two terms, I tried to compare them with the results that I was getting for the correct part of issue #25682 i.e

In [5]: x = Symbol('x', negative=True)

In [6]: (1/abs(sqrt(1 - (-1 + 1/x)**2))).series(x)
Out[6]: 
             3      4       5        
      2   3⋅x    5⋅x    35⋅x     ⎛ 6⎞
-x - x  - ──── - ──── - ───── + O⎝x ⎠
           2      2       8          

the above is correct according to wolfram alpha, now digging into the working of sympy I saw that abs(sqrt(1 - (-1 + 1/x)**2)).series(x) was computed first followed by inverting it to get the required result so what I noticed the following:

In [7]: abs(sqrt(1 - (-1 + 1/x)**2)).series(x)
Out[7]: 
               2      3      4       5        
  1       x   x    5⋅x    7⋅x    21⋅x     ⎛ 6⎞
- ─ + 1 + ─ + ── + ──── + ──── + ───── + O⎝x ⎠
  x       2   2     8      8      16          

In [8]: (sqrt(1 - (-1 + 1/x)**2)).series(x)
Out[8]: 
                   2        3        4         5        
  ⅈ       ⅈ⋅x   ⅈ⋅x    5⋅ⅈ⋅x    7⋅ⅈ⋅x    21⋅ⅈ⋅x     ⎛ 6⎞
- ─ + ⅈ + ─── + ──── + ────── + ────── + ─────── + O⎝x ⎠
  x        2     2       8        8        16           

so in the results above we can compare the first few terms of abs(expr).series(x) and expr.series(x) (note the all the above shown results are valid in the current master and also in the PR branch). Now according to the process mentioned for in the referenced comment that is for a + b*x**e we have Abs(a) + re(b/sign(a)*cdir**e)*cdir**-e*x**e clearly does hold here. Now lets do a similar workout for the other branch i.e

In [9]: y = symbols('y', positive = True)

In [10]: ( 1/abs(sqrt(1 - (-1 + 1/y)**2))).series(y)
Out[10]: 
            3      4       5        
     2   3⋅y    5⋅y    35⋅y     ⎛ 6⎞
y + y  + ──── + ──── + ───── + O⎝y ⎠
          2      2       8          

(NOTE: the above expression is correct according to wolframbut is not currently given in master but in PR branch.) Similarly for the above if we look at the corresponding series expansion for the denominator of the expression then we get the following(NOTE : again the following is in the PR branch and not in the master)

In [11]: abs(sqrt(1 - (-1 + 1/y)**2)).series(y)
Out[11]: 
             2      3      4       5        
1       y   y    5⋅y    7⋅y    21⋅y     ⎛ 6⎞
─ - 1 - ─ - ── - ──── - ──── - ───── + O⎝y ⎠
y       2   2     8      8      16          

In [12]: (sqrt(1 - (-1 + 1/y)**2)).series(y)
Out[12]: 
                 2        3        4         5        
ⅈ       ⅈ⋅y   ⅈ⋅y    5⋅ⅈ⋅y    7⋅ⅈ⋅y    21⋅ⅈ⋅y     ⎛ 6⎞
─ - ⅈ - ─── - ──── - ────── - ────── - ─────── + O⎝y ⎠
y        2     2       8        8        16           

now what I intend to say is that for out[10] to be what it is we need out[11] as shown(both 11 and 10 have been corrected results in the pr branch and not valid in master) but out[12] is given by both pr and master, and thus now if we again compare the out[11] and out[12] in the light of expr.series(x) being this a + b*x**e then abs(expr).series(x) will be Abs(a) + re(b/sign(a))*x**e, things don't hold true.
I think most probably am missing something here or Out[12] is wrong here, but am also not able to verify them(Out[8] and Out[12]) s as wolfram fails to calculate them. The solution proposed here in this PR is quite observational in that regard.

[anutosh491]

The tests look good though I'll have to think through this. I remember while I was appraoching this a couple of years back I ended up justifying this return (s/sign(direction)).expand() to be the correct return statement finally.

[arnabnandikgp]

Is there a need to add my name again in the mailmap file? As running bin/mailmap_check.py locally says No changes needed in .mailmap though.