Opened 5 years ago

# Extend normalize() and use it instead of Maxima in simplify_rational()

Reported by: Owned by: rws major sage-7.4 symbolics Ralf Stephan N/A u/rws/noexpand_simple_options_for_ex_normalize__ 30bc79e56390859ee732a0d08152cb93bc7528ed #21369, #21529

At the moment `normalize()` will not expand factors of fractions. If fractions are combined however, the final numerator is expanded:

```sage: ((x^(y/2) + 1)^2*(x^(y/2) - 1)^2/(x^y - 1)).normalize()
(x^(1/2*y) + 1)^2*(x^(1/2*y) - 1)^2/(x^y - 1)
sage: (x + y^2/(x + 2)).normalize()
(x^2 + y^2 + 2*x)/(x + 2)
```

The alternatives are provided atm by Maxima

```sage: ((x^(y/2) + 1)^2*(x^(y/2) - 1)^2/(x^y - 1)).simplify_rational(algorithm='simple')
(x^(2*y) - 2*x^y + 1)/(x^y - 1)
sage: (x + y^2/(x + 2)).simplify_rational(algorithm='noexpand')
((x + 2)*x + y^2)/(x + 2)
```

After Pynac will have implemented `normalize()` options to these effect the calls to Maxima in `simplify_rational` should be replaced by the respective calls to Pynac.

### comment:1 Changed 5 years ago by rws

• Description modified (diff)

### comment:2 Changed 5 years ago by rws

• Dependencies set to #21360

This is implemented in Pynac master but one doctest depends on #21360. Also, nested application ("full") needs to be done.

Last edited 5 years ago by rws (previous) (diff)

### comment:3 Changed 5 years ago by rws

• Dependencies #21360 deleted

### comment:4 Changed 5 years ago by rws

• Branch set to u/rws/noexpand_simple_options_for_ex_normalize__

### comment:5 Changed 5 years ago by rws

• Authors set to Ralf Stephan
• Commit set to 129b6e6ba83e18ee5b2cdbc2d3caac72d5f977f2
• Dependencies set to #21369

New commits:

 ​b996f4f `version/chksum` ​91a08d2 `changes affecting Sage behaviour or interface` ​a4f58d7 `doctest fixes` ​ac7d971 `revert removal of pos.char. doctests; add "known bug"` ​3dd8058 `address reviewer's comments` ​1f29305 `add doctest` ​0065f62 `address reviewer's comments` ​129b6e6 `use normal() instead of Maxima`

### comment:6 Changed 5 years ago by git

• Commit changed from 129b6e6ba83e18ee5b2cdbc2d3caac72d5f977f2 to c69fd2e30ddbbcd21af73f79920d774826f42d46

Branch pushed to git repo; I updated commit sha1. New commits:

 ​b06cfa0 `Fix _eval_self(float) for "real" complex expressions` ​fd9fa19 `Merge branch 'u/jdemeyer/update_to_pynac_0_6_9' of trac.sagemath.org:sage into t/21335/noexpand_simple_options_for_ex_normalize__` ​c69fd2e `21335: fullratsimp: replace calls to Maxima with Pynac ones`

### comment:7 Changed 5 years ago by rws

Three doctest fails in `symbolic/expression.pyx`.

### comment:8 Changed 5 years ago by rws

• Summary changed from Noexpand/simple options for ex.normalize() to Extend normalize() and use it instead of Maxima in simplify_rational()

### comment:9 Changed 5 years ago by rws

• Dependencies changed from #21369 to #21369, #21529

### comment:10 Changed 5 years ago by git

• Commit changed from c69fd2e30ddbbcd21af73f79920d774826f42d46 to c4932f87e29a6122b0e5c99eaf9499809a365c49

Branch pushed to git repo; I updated commit sha1. New commits:

 ​a8eba31 `Merge branch 'develop' into t/21335/noexpand_simple_options_for_ex_normalize__` ​2ea5c31 `21529: fix bug in factoring of general symbolic expressions` ​c4932f8 `Merge branch 'u/rws/bug_in_factoring_of_general_symbolic_expressions' of trac.sagemath.org:sage into t/21335/noexpand_simple_options_for_ex_normalize__`

### comment:11 Changed 5 years ago by rws

• Status changed from new to needs_review

### comment:12 Changed 5 years ago by mmezzarobba

Minor point: "and and" should be "and".

Apart from that, I don't understand why `algorithm="full"` attempts to factor the simplified fraction (which is a potentially costly operation). Is there something in the way the Pynac `normal()` function works that makes it necessary to do so?

### comment:13 follow-up: ↓ 15 Changed 5 years ago by rws

Indeed the performance question was the source of some headaches to me, as well. I figured at the time consistency with Maxima behaviour was more important. Without `factor` the following doctests will fail:

```File "src/sage/symbolic/expression.pyx", line 9427, in sage.symbolic.expression.Expression.simplify_rational
Failed example:
f.simplify_rational()
Expected:
-2*sqrt(x - 1)/sqrt((x + 1)*(x - 1))
Got:
((x - 1)^(3/2) - sqrt(x - 1)*x - sqrt(x - 1))/sqrt((x + 1)*(x - 1))
**********************************************************************
File "src/sage/symbolic/expression.pyx", line 9451, in sage.symbolic.expression.Expression.simplify_rational
Failed example:
g.simplify_rational()
Expected:
x^y - 1
Got:
(x^(2*y) - 2*x^y + 1)/(x^y - 1)
```

### comment:14 Changed 5 years ago by git

• Commit changed from c4932f87e29a6122b0e5c99eaf9499809a365c49 to 30bc79e56390859ee732a0d08152cb93bc7528ed

Branch pushed to git repo; I updated commit sha1. New commits:

 ​ab551be `Merge branch 'develop' into t/21335/noexpand_simple_options_for_ex_normalize__` ​30bc79e `cosmetics`

### comment:15 in reply to: ↑ 13 ; follow-up: ↓ 16 Changed 5 years ago by mmezzarobba

Indeed the performance question was the source of some headaches to me, as well. I figured at the time consistency with Maxima behaviour was more important.

I don't know exactly what either Maxima or Pynac does, but just by chance, do you think it would be okay to keep the loop but expand the denominators instead of calling `factor()`?

### comment:16 in reply to: ↑ 15 Changed 5 years ago by rws

I don't know exactly what either Maxima or Pynac does, but just by chance, do you think it would be okay to keep the loop but expand the denominators instead of calling `factor()`?

This will, additionally to the two doctests above, fail this doctest:

```File "src/sage/symbolic/expression.pyx", line 9458, in sage.symbolic.expression.Expression.simplify_rational
Failed example:
f.simplify_rational()
Expected:
(2*x^2 + 5*x + 4)/((x + 2)^2*(x + 1))
Got:
(2*x^2 + 5*x + 4)/(x^3 + 5*x^2 + 8*x + 4)
```

### comment:17 Changed 5 years ago by rws

• Status changed from needs_review to needs_work

I think I see now how to resolve the three tests without factoring. Pynac's gcd needs to be a bit more aware of powers in the expression and needs to replace some of them with new symbols. This way also exponentials can be handled identically (which does not work atm).

Note: See TracTickets for help on using tickets.