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

Deprecate mdft #20246

Open
oscargus opened this issue Oct 11, 2020 · 7 comments
Open

Deprecate mdft #20246

oscargus opened this issue Oct 11, 2020 · 7 comments
Labels
Deprecation Removal Tracks the removal of a deprecated feature. See github.com/sympy/sympy/wiki/Deprecating-policy physics

Comments

@oscargus
Copy link
Contributor

oscargus commented Oct 11, 2020

There is a fully functioning DFT class in matrices.expressions.fourier, so the mdft method in physics.matrices should be deprecated.

If one would like identical behavior (an evaluated matrix), one should replace mdft(4) with DFT(4).as_explicit().

This is the deprecation removal tracking issue.

@oscargus oscargus added the Deprecation Removal Tracks the removal of a deprecated feature. See github.com/sympy/sympy/wiki/Deprecating-policy label Oct 11, 2020
@oscarbenjamin
Copy link
Contributor

Hi @oscargus!

That seems reasonable. Could you expand a bit on how each of these would be used and how the output would look?

@oscargus
Copy link
Contributor Author

Not sure if I fully understand the question, but here is an example:

In [1]: from sympy import init_printing

In [2]: init_printing(use_latex=False)

In [3]: from sympy.physics.matrices import mdft

In [4]: mdft(3)
Out[4]: 
⎡√3      √3           √3     ⎤
⎢──      ──           ──     ⎥
⎢3       3            3      ⎥
⎢                            ⎥
⎢        -2⋅ⅈ⋅π        2⋅ⅈ⋅π ⎥
⎢        ───────       ───── ⎥
⎢           3            3   ⎥
⎢√3  √3⋅ℯ          √3⋅ℯ      ⎥
⎢──  ───────────   ───────── ⎥
⎢3        3            3     ⎥
⎢                            ⎥
⎢         2⋅ⅈ⋅π       -2⋅ⅈ⋅π ⎥
⎢         ─────       ───────⎥
⎢           3            3   ⎥
⎢√3   √3⋅ℯ        √3⋅ℯ       ⎥
⎢──   ─────────   ───────────⎥
⎣3        3            3     ⎦

In [5]: from sympy.matrices.expressions.fourier import DFT

In [6]: DFT(3)
Out[6]: DFT(3)

In [7]: DFT(3).as_explicit()
Out[7]: 
⎡√3      √3           √3     ⎤
⎢──      ──           ──     ⎥
⎢3       3            3      ⎥
⎢                            ⎥
⎢        -2⋅ⅈ⋅π        2⋅ⅈ⋅π ⎥
⎢        ───────       ───── ⎥
⎢           3            3   ⎥
⎢√3  √3⋅ℯ          √3⋅ℯ      ⎥
⎢──  ───────────   ───────── ⎥
⎢3        3            3     ⎥
⎢                            ⎥
⎢         2⋅ⅈ⋅π       -2⋅ⅈ⋅π ⎥
⎢         ─────       ───────⎥
⎢           3            3   ⎥
⎢√3   √3⋅ℯ        √3⋅ℯ       ⎥
⎢──   ─────────   ───────────⎥
⎣3        3            3     ⎦

As seen, the output of mdft(3) and DFT(3).as_explicit() is identical. As far as I can tell, mdft does not provide any additional advantages over DFT, so it makes sense to keep DFT only.

@oscargus
Copy link
Contributor Author

For use cases, I use them every now and then when I need a DFT matrix (and not computing the DFT of an input sequence through FFT).

There is also the related issue of how to scale these matrices and the direction of the phase shift for DFT and FFT, see #15804. But that is a different issue. The current deprecation is simply a way to keep identical code in a single place.

@oscarbenjamin
Copy link
Contributor

oscarbenjamin commented Oct 12, 2020

here is an example

Yes, that's what I meant. The deprecation warning will point people to come and look at the information here so showing the alternatives and how to change their code is useful.

@oscargus
Copy link
Contributor Author

It has been pointed out that DFT(3).as_explicit() is not identical to mdft(3) as DFT(3).as_explicit() is not mutable. To get a mutable matrix use DFT(3).as_mutable().

@AntiPhoton47
Copy link
Contributor

Hi, not sure what the status of this issue is, but I noticed that there is no entry for sympy.matrices.expressions.fourier in the API reference documentation.

@asmeurer
Copy link
Member

There's a lot of bits and pieces of docstrings that are missing from the API documentation. Someone needs to go through and add them all (ideally by replacing explicit autofunction lists with automodule + :members: so that it automatically grabs everything). One issue is that many docstrings that aren't presently included in the docs have invalid RST because they were never built, so it is often required to go through and clean up some minor things when doing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecation Removal Tracks the removal of a deprecated feature. See github.com/sympy/sympy/wiki/Deprecating-policy physics
Projects
None yet
Development

No branches or pull requests

4 participants