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

polynom division with refine #17634

Open
TSlivede opened this issue Sep 19, 2019 · 11 comments
Open

polynom division with refine #17634

TSlivede opened this issue Sep 19, 2019 · 11 comments
Labels

Comments

@TSlivede
Copy link

I tried to do some simplification (on live.sympy.org) with refine, but couldn't get it to work. It seems to boil down to this:

refine((1-x**2)/(1-x),Q.real(x))

gives back (1-x**2)/(1-x),
while

simplify((1-x**2)/(1-x))

correctly simplifies it to x+1.

Maybe I'm just not understanding this correctly, as I'm relatively new to sympy - but shouldn't refine also simplify that expression?

I want to use refine because I actually have expressions of the form

refine(sqrt(1-x**2)/sqrt(1-x), Q.positive(1-x) & Q.positive(1-x**2))

where I don't think I can use simplify, because I can't tell sympy, that the root is positive.

@oscarbenjamin
Copy link
Contributor

You can do this if x is positive using factor:

In [21]: x = Symbol('x', positive=True)                                                                                           

In [22]: sqrt(1-x**2)/sqrt(1-x)                                                                                                   
Out[22]: 
   ________
  ╱      2 
╲╱  1 - x  
───────────
   _______ 
 ╲╱ 1 - x  

In [23]: factor(_)                                                                                                                
Out[23]: 
  _______
╲╱ x + 1 

I'm not sure why x being positive makes this work.

In general I think there should be a function for doing sqrt(a)/sqrt(b) -> sqrt(a/b). Maybe powsimp force=True should do it. This works for arbitrary y:

In [38]: e = x**y/z**y                                                                                                            

In [39]: e                                                                                                                        
Out[39]: 
 y  -y
x z  

In [40]: powsimp(e)                                                                                                               
Out[40]: 
 y  -y
x z  

In [41]: powsimp(e, force=True)                                                                                                   
Out[41]: 
   y
⎛x⎞ 
⎜─⎟ 
⎝z⎠ 

However if y = 1/2 then it fails:

In [42]: e2 = e.subs(y, S(1)/2)                                                                                                   

In [43]: e2                                                                                                                       
Out[43]: 
x
──
z

In [45]: powsimp(e2)                                                                                                              
Out[45]: 
x
──
z

In [46]: powsimp(e2, force=True)                                                                                                  
Out[46]: 
x
──
z

@oscarbenjamin
Copy link
Contributor

It seems that powsimp calls itself recursively and the difference above comes from

In [8]: powsimp(z**-S.Half, force=True)                                                                                                        
Out[8]: 
1 
──
z

In [9]: powsimp(z**-y, force=True)                                                                                                             
Out[9]: 
   y
⎛1⎞ 
⎜─⎟ 
⎝z⎠

Maybe it should be changed so that we have:

In [8]: powsimp(z**-S.Half, force=True)                                                                                                        
Out[8]: 
    ___
   ╱ 1 
  ╱  ─ 
╲╱   z 

I.e. powsimp(1/sqrt(z), force=True) -> sqrt(1/z).

@oscarbenjamin
Copy link
Contributor

Looks like the difference is here:

exp_c, exp_t = e.as_coeff_Mul(rational=True)
if exp_c is not S.One and exp_t is not S.One:
c_powers[i] = [Pow(b, exp_c), exp_t]

since we have:

In [1]: (-y).as_coeff_Mul()                                                                                                                    
Out[1]: (-1, y)

In [2]: (-S.Half).as_coeff_Mul()                                                                                                               
Out[2]: (-1/2, 1)

@oscarbenjamin
Copy link
Contributor

This makes it possible to use powsimp:

diff --git a/sympy/simplify/powsimp.py b/sympy/simplify/powsimp.py
index baed0aac83..e0208e17a5 100644
--- a/sympy/simplify/powsimp.py
+++ b/sympy/simplify/powsimp.py
@@ -393,6 +393,8 @@ def update(b):
             if not (all(x.is_nonnegative for x in b.as_numer_denom()) or e.is_integer or force or b.is_polar):
                 continue
             exp_c, exp_t = e.as_coeff_Mul(rational=True)
+            if exp_t is S.One and exp_c.is_negative:
+                exp_c, exp_t = -exp_t, -exp_c
             if exp_c is not S.One and exp_t is not S.One:
                 c_powers[i] = [Pow(b, exp_c), exp_t]

Then we have:

In [2]: powsimp(sqrt(1-x**2)/sqrt(1-x), force=True)                                                                                            
Out[2]: 
     ________
    ╱      21 - x  
  ╱   ────── 
╲╱    1 - x  

However I don't see how to simplify it from there...

In [3]: factor(_)                                                                                                                              
Out[3]: 
    ___________________
   ╱ -(x - 1)(x + 1)  
  ╱  ───────────────── 
╲╱         1 - x       

In [4]: cancel(_)                                                                                                                              
Out[4]: 
     _________________
    ╱      2          
   ╱      x       1- ───── + ───── 
╲╱      1 - x   1 - x 

In [6]: simplify(_)                                                                                                                            
Out[6]: 
     ________
    ╱  2     
   ╱  x  - 1 
  ╱   ────── 
╲╱    x - 1  

@oscarbenjamin
Copy link
Contributor

Okay here it is (using the diff above):

In [1]: powsimp(sqrt(1-x**2)/sqrt(1-x), force=True)                                                                                            
Out[1]: 
     ________
    ╱      21 - x  
  ╱   ────── 
╲╱    1 - x  

In [2]: factor(_, deep=True)                                                                                                                   
Out[2]: 
  _______
╲╱ x + 1 

If anyone wants to add that diff (plus tests) as a PR then go ahead.

@oscarbenjamin oscarbenjamin added the Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. label Sep 20, 2019
@arun-y99
Copy link
Contributor

I'll add the diff and tests

@espatatis
Copy link

Can I get this issue assigned ?

@oscarbenjamin
Copy link
Contributor

We don't assign issues but you can work on this if you want to. There already is a PR #17666 which you could use as a starting point.

@AnjaliJha167
Copy link

If no one is working on this issue and the issue is still open. Can I start working on it?
Also, it would be great, if anyone could guide me as to where to start from

@oscarbenjamin
Copy link
Contributor

I would start by looking at the two existing PRs (#18031 and #17666) to see why they didn't work. Apparently this isn't as easy as I first thought.

@oscarbenjamin oscarbenjamin removed the Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. label Jan 5, 2020
@czgdp1807
Copy link
Member

The issue still persists on master. Open for work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants