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

integrate() does not work with multivariable function that is solved by simple substitution. DomainError: there is no ring associated with CC #21741

Closed
LuisMi1245 opened this issue Jul 14, 2021 · 15 comments · Fixed by #21750
Labels
Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. integrals.risch integrals

Comments

@LuisMi1245
Copy link

LuisMi1245 commented Jul 14, 2021

>> x, y, z, t = symbols('x y z t')
>> fun = E ** ( (-2*I*pi* (z*x+t*y) / (500*(10**(-9))) ) )
returns the following:
image

Now I try to integrate with respect to z:
>> integrate(fun, z)
And the output is an error raised by the get_ring() method of the Domain class (I think) that says:

DomainError: there is no ring associated with CC

However, if I rewrite the function obtained by python fun, but in this case, removing the decimal point and the leftover zero in a new sympy variable fun2, then the integral solves beautifully. I mean this:

>> fun2 = E**( -4000000*I*pi*(t*y+x*z) )
>> integrate(fun2,z)
returns the following:
image

And this is correct. Anyway, I had to spend hours trying to understand the reason for the error, although I honestly don't know the reason. I can only conclude that the integration system might have some internal bug.

@oscarbenjamin
Copy link
Contributor

I see this error with sympy 1.8 but it is already fixed on master. I've bisected the fix to b963ae3 from #21337.

I'm marking this as easy to fix. This issue can be closed if a test is added for the two examples above in sympy/integrals/tests/test_integrals.py.

@oscarbenjamin oscarbenjamin added Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. integrals integrals.risch labels Jul 14, 2021
@praneethratna
Copy link
Contributor

@oscarbenjamin Hi , I'm kinda new to open source contribution and would like to work on fixing this issue , can i be assigned this issue?

@oscarbenjamin
Copy link
Contributor

Issues are not assigned to anyone. I have marked it as easy to fix in order to invite someone to contribute a PR that fixes this so go ahead. There is a guide to the workflow here:
https://github.com/sympy/sympy/wiki/Development-workflow

@praneethratna
Copy link
Contributor

@oscarbenjamin Hey , I'm kinda struck on how to write the test case for this issue , could you please give a headstart on how to write?

@LuisMi1245
Copy link
Author

@praneethratna I think what Oscar is saying is that you need to add in the integral test method stack (sympy/integrals/tests/test_integrals.py), the two forms of the integral that I mention here with the correct solutions, using the assert function to throw exceptions in case the condition is not met due to problems with the integration algorithm.

@LuisMi1245
Copy link
Author

@oscarbenjamin The error has not yet been resolved. I added the two integrals to the test_integrals.py file following the examples in that file. On the other hand, when running:

>> ./bin/test sympy/integrals/tests/test_integrals.py

The test fails as shown in the image. I use the most recent version (from 2 hours ago) from the sympy repository on Github.
image

What else do you recommend doing?

@LuisMi1245
Copy link
Author

This is the same code. And the bin/test suggests me: "DO NOT COMMIT".

image

@oscarbenjamin
Copy link
Contributor

Not sure what the problem is but try running that code directly outside of the test suite and see what the output is for both lhs and rhs.

Note that 10**-9 will create a float rather than Rational

@LuisMi1245
Copy link
Author

Not sure what the problem is but try running that code directly outside of the test suite and see what the output is for both lhs and rhs.

Note that 10**-9 will create a float rather than Rational

But the integration method should support the coefficient being of rational or float type, right?

@LuisMi1245
Copy link
Author

LuisMi1245 commented Jul 14, 2021

You are right, now it does work and performs the integral without problems (outside the test environment with the latest version of the repository), however the results are not the same even though the expressions to integrate are the same in the mathematical context. I think it has to do with what you mention about the 10**(-9).

image

@praneethratna
Copy link
Contributor

@LuisMi1245 i got the same error that you got while adding the test case , well as also mentioned this seems to be a problem with 10**(-9) being a float but how can that be fixed?

@LuisMi1245
Copy link
Author

LuisMi1245 commented Jul 15, 2021

@praneethratna

def test_issue_21741():
    x,y,z,t = symbols('x y z t', positive=True)
    #Both integrals are exactly the same. 
    assert integrate(E ** ((-2*I*pi*(z*x+t*y))/((500*(Rational(1,10**9))))), z).simplify() - (I*E**(-4000000*I*pi*(t*y+x*z)))/(4000000*pi*x).simplify() == 0
    assert integrate((E ** (-4000000*I*pi*(z*x+t*y))), z).simplify() - (I*E**(-4000000*I*pi*(t*y+x*z)))/(4000000*pi*x).simplify() == 0

You just need to understand that 10**-9 is a division that will first be captured and evaluated by python as a floating point expression. In that case, you must indicate that the expression is of type sympy using simpify() or Rational(1,10**9) so that it retains the fraction. You can find this at:
Sympy: Simbolic-Expressions

@oscarbenjamin
Copy link
Contributor

The floating point numbers need to match exactly for the test to succeed:

In [45]: a = Float('3999999.9999999995', precision=53)                                                                                                        

In [46]: b = Float('2.5000000000000004e-7', precision=53)                                                                                                     

In [47]: r = Piecewise((b*I*exp(-a*I*pi*t*y)*exp(-a*I*pi*x*z)/(pi*x), Ne(1.0*pi*x*exp(a*I*pi*t*y), 0)), (z*exp(-a*I*pi*t*y), True))                           

In [48]: integrate(fun, z) == r                                                                                                                               
Out[48]: True

You can see the internal representation of the expressions in full detail using srepr:

In [49]: srepr(integrate(fun, z))                                                                                                                             
Out[49]: "Piecewise(ExprCondPair(Mul(Float('2.5000000000000004e-7', precision=53), I, Pow(pi, Integer(-1)), Pow(Symbol('x'), Integer(-1)), exp(Mul(Integer(-1), Float('3999999.9999999995', precision=53), I, pi, Symbol('t'), Symbol('y'))), exp(Mul(Integer(-1), Float('3999999.9999999995', precision=53), I, pi, Symbol('x'), Symbol('z')))), Unequality(Mul(Float('1.0', precision=53), pi, Symbol('x'), exp(Mul(Float('3999999.9999999995', precision=53), I, pi, Symbol('t'), Symbol('y')))), Integer(0))), ExprCondPair(Mul(Symbol('z'), exp(Mul(Integer(-1), Float('3999999.9999999995', precision=53), I, pi, Symbol('t'), Symbol('y')))), true))"

There should probably be a helper function for tests that can check whether two expressions are almost equal wrt floating point numbers.

@smichr
Copy link
Member

smichr commented Jul 15, 2021

should probably be a helper function for tests that can check whether two expressions are almost equal wrt floating point

>>> a = Float(4*10**6)
>>> b = Float(2.5e-7)
>>> r = Piecewise((b*I*exp(-a*I*pi*t*y)*exp(-a*I*pi*x*z)/(pi*x), Ne(1.0*pi*x*exp
(a*I*pi*t*y), 0)), (z*exp(-a*I*pi*t*y), True))
>>> ans = integrate(fun,z)
>>> r==ans
False
>>> str(r)==str(ans)
True
>>> dif = ans - r
>>> dif.xreplace({i:Float(str(i)) for i in dif.atoms(Float)})
0

@LuisMi1245
Copy link
Author

The floating point numbers need to match exactly for the test to succeed:

In [45]: a = Float('3999999.9999999995', precision=53)                                                                                                        

In [46]: b = Float('2.5000000000000004e-7', precision=53)                                                                                                     

In [47]: r = Piecewise((b*I*exp(-a*I*pi*t*y)*exp(-a*I*pi*x*z)/(pi*x), Ne(1.0*pi*x*exp(a*I*pi*t*y), 0)), (z*exp(-a*I*pi*t*y), True))                           

In [48]: integrate(fun, z) == r                                                                                                                               
Out[48]: True

You can see the internal representation of the expressions in full detail using srepr:

In [49]: srepr(integrate(fun, z))                                                                                                                             
Out[49]: "Piecewise(ExprCondPair(Mul(Float('2.5000000000000004e-7', precision=53), I, Pow(pi, Integer(-1)), Pow(Symbol('x'), Integer(-1)), exp(Mul(Integer(-1), Float('3999999.9999999995', precision=53), I, pi, Symbol('t'), Symbol('y'))), exp(Mul(Integer(-1), Float('3999999.9999999995', precision=53), I, pi, Symbol('x'), Symbol('z')))), Unequality(Mul(Float('1.0', precision=53), pi, Symbol('x'), exp(Mul(Float('3999999.9999999995', precision=53), I, pi, Symbol('t'), Symbol('y')))), Integer(0))), ExprCondPair(Mul(Symbol('z'), exp(Mul(Integer(-1), Float('3999999.9999999995', precision=53), I, pi, Symbol('t'), Symbol('y')))), true))"

There should probably be a helper function for tests that can check whether two expressions are almost equal wrt floating point numbers.

Yes, I gave up trying a PR due to ignorance. I am glad I understood.

skirpichev added a commit to skirpichev/diofant that referenced this issue Jul 16, 2021
skirpichev added a commit to skirpichev/diofant that referenced this issue Jul 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. integrals.risch integrals
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants