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

Bug in fourier_transform #22787

Open
mathcube7 opened this issue Jan 3, 2022 · 11 comments
Open

Bug in fourier_transform #22787

mathcube7 opened this issue Jan 3, 2022 · 11 comments

Comments

@mathcube7
Copy link

both the functions fourier_transform and _fourier_transform give the (same but) wrong result, when applied to the function sin(x)**2/x**2. Here is the sample code:

from sympy.abc import *
from sympy import *
from sympy.integrals.transforms import _fourier_transform, fourier_transform

f = sin(x)**2/x**2
F = _fourier_transform(f, x, k, a=1/(2*pi), b=-1, name='F')
plot(F, (k, -3, 3))

image

or in formula

image

This cannot be true, however. As f is real, the Fourier transform must satisfy F*(k) = F(-k). And since the Fourier transform itself is real, it must be even: F(k)=F(-k). Obviously, the result of sympy is not.

Sympy version used: 1.9

@theartpiece
Copy link

can I be assigned @oscarbenjamin

@anutosh491
Copy link
Member

@theartpiece issues are not generally assigned in sympy . If you want to try it out surely you could go forward and if you face any difficulties during debugging or don't understand something you can comment that here .

@theartpiece
Copy link

@mathcube7 in general this function seems to behave incorrectly.
Like Try f=e^(-|t|)

@theartpiece
Copy link

yeah so basically the problem lies here

F, cond = F.args[0]
.
First of all, the problem occurs only when the transformation returns a piecewise function. In this case, fourier returns the first part of the piecewise function (where I think the author assumed that the first part corresponds to the positive case, i.e., x>0 ).
SUGGESION- Add another function f in the return output such that f(x)=f(-x).

@theartpiece
Copy link

@oscarbenjamin Let me know if it makes sense and I'll make a PR.

@oscarbenjamin
Copy link
Contributor

That code definitely looks suspect. In this case there are nested Piecewise so piecewise_fold should probably be used. Clearly also the conditions of the Piecewise should be checked in some way before simply choosing the first case. Probably the author expected that the condition would be on symbolic parameters rather than the Fourier variable but that should be checked rather than assumed.

@theartpiece
Copy link

How do you add unit tests?

@mostlyaman
Copy link
Contributor

mostlyaman commented Jan 5, 2022

How do you add unit tests?

Tests are added in the corresponding test file for the file in which the change was made. for integrals/transforms.py, the tests are added in integrals/tests/test_transforms.py
add a function referencing the issue number which led to the change and add the the tests in that function.
Take this for an example.Add your tests at the END of the test file.

def test_issue_7181():
assert mellin_transform(1/(1 - x), x, s) != None

I dont think there is a guide for adding unit tests in the Development Workflow. Should I add that to the Development Workflow wiki?

@oscargus
Copy link
Contributor

oscargus commented Jan 7, 2022

There are:
https://github.com/sympy/sympy/wiki/Writing-tests
https://github.com/sympy/sympy/wiki/Writing-test-cases-in-SymPy

(The problem is probably more that there are 592 wiki-pages...)

@oscarbenjamin
Copy link
Contributor

(The problem is probably more that there are 592 wiki-pages...)

Maybe the dev workflow could link to the other pages. Probably a lot of stuff could be stripped out of the dev workflow and just replaced with "here's a link for more information if needed".

@mostlyaman
Copy link
Contributor

Probably a lot of stuff could be stripped out of the dev workflow and just replaced with "here's a link for more information if needed".

Yes, the dev workflow contains too much content and it may be difficult for a new contributor to use. And stuff like Writing tests and Adding someone to AUTHORS should be a part of Dev workflow as a new contributor would need to do these things. The wiki page has got too long to easily navigate too.

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

No branches or pull requests

6 participants