added rewrite for trigonometric functions as Besselj #24387

Ishanned · 2022-12-14T14:44:01Z

References to other Issues or PRs

resolves #24176

Brief description of what is fixed or changed

As mentioned, trigonometric functions were rewritten as Besselj.
Corresponding tests were also added.

Other comments

Release Notes

functions
- added rewrite for trigonometric functions as BesselJ.

sympy-bot · 2022-12-14T14:44:04Z

✅

Hi, I am the SymPy bot (version not found!). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

functions
- added rewrite for trigonometric functions as BesselJ. (#24387 by @Ishanned)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.12.

Click here to see the pull request description that was parsed.

<!-- Your title above should be a short description of what
was changed. Do not include the issue number in the title. -->

#### References to other Issues or PRs
<!-- If this pull request fixes an issue, write "Fixes #NNNN" in that exact
format, e.g. "Fixes #1234" (see
https://tinyurl.com/auto-closing for more information). Also, please
write a comment on that issue linking back to this pull request once it is
open. -->
resolves #24176 

#### Brief description of what is fixed or changed
As mentioned, trigonometric functions were rewritten as Besselj. 
Corresponding tests were also added. 

#### Other comments


#### Release Notes

<!-- Write the release notes for this release below between the BEGIN and END
statements. The basic format is a bulleted list with the name of the subpackage
and the release note for this PR. For example:

* solvers
  * Added a new solver for logarithmic equations.

* functions
  * Fixed a bug with log of integers.

or if no release note(s) should be included use:

NO ENTRY

See https://github.com/sympy/sympy/wiki/Writing-Release-Notes for more
information on how to write release notes. The bot will check your release
notes automatically to see if they are formatted correctly. -->

<!-- BEGIN RELEASE NOTES -->
* functions
  * added rewrite for trigonometric functions as BesselJ. 
<!-- END RELEASE NOTES -->

github-actions · 2022-12-15T10:32:52Z

Benchmark results from GitHub Actions

Lower numbers are good, higher numbers are bad. A ratio less than 1
means a speed up and greater than 1 means a slowdown. Green lines
beginning with + are slowdowns (the PR is slower then master or
master is slower than the previous release). Red lines beginning
with - are speedups.

Significantly changed benchmark results (PR vs master)

Significantly changed benchmark results (master vs previous release)

       before           after         ratio
     [41d90958]       [c7fe006d]
     <sympy-1.11.1^0>                 
-        2.47±0ms      1.09±0.07ms     0.44  solve.TimeSparseSystem.time_linear_eq_to_matrix(20)
-     4.93±0.01ms       1.61±0.1ms     0.33  solve.TimeSparseSystem.time_linear_eq_to_matrix(30)

Full benchmark results can be found as artifacts in GitHub Actions
(click on checks at the top of the PR).

Ishanned · 2022-12-15T19:10:09Z

@oscarbenjamin can you have a look?

oscarbenjamin · 2022-12-15T20:21:30Z

@oscarbenjamin can you have a look?

There are many people who could review this. It is usually better not to reference a particular person because it tends to discourage others from reviewing.

Ishanned · 2022-12-16T11:50:40Z

Will surely keep that in mind!

oscarbenjamin · 2023-01-14T22:59:31Z

This PR needs to be reviewed. From a very quick skim of the diff it looks reasonable but I haven't checked anything in any detail.

asmeurer · 2023-04-25T20:31:29Z

sympy/functions/elementary/trigonometric.py

@@ -532,6 +532,10 @@ def _eval_rewrite_as_sec(self, arg, **kwargs):
    def _eval_rewrite_as_sinc(self, arg, **kwargs):
        return arg*sinc(arg)

+    def _eval_rewrite_as_besselj(self, arg, **kwargs):
+        from sympy.functions.special.bessel import besselj
+        return sqrt(pi*arg/2)*besselj(1*S.Half,arg)


You don't need to write 1*.

asmeurer · 2023-04-25T20:31:39Z

sympy/functions/elementary/trigonometric.py

@@ -943,6 +947,10 @@ def _eval_rewrite_as_sec(self, arg, **kwargs):
    def _eval_rewrite_as_csc(self, arg, **kwargs):
        return 1/sec(arg).rewrite(csc)

+    def _eval_rewrite_as_besselj(self, arg, **kwargs):
+        from sympy.functions.special.bessel import besselj
+        return sqrt(pi*arg/2)*besselj(-1*S.Half,arg)


Same here. This can just be -S.Half.

asmeurer · 2023-04-25T20:31:58Z

sympy/functions/elementary/trigonometric.py

@@ -1298,6 +1306,10 @@ def _eval_rewrite_as_sqrt(self, arg, **kwargs):
            return None
        return y

+    def _eval_rewrite_as_besselj(self, arg, **kwargs):
+        from sympy.functions.special.bessel import besselj
+        return (sin(arg).rewrite(besselj)/cos(arg).rewrite(besselj))


Pass **kwargs through to recursive rewrite calls.

asmeurer · 2023-04-25T20:41:42Z

The formulae here appear to be correct https://dlmf.nist.gov/10.16.

asmeurer · 2023-04-25T20:42:29Z

sympy/functions/elementary/tests/test_trigonometric.py

@@ -217,6 +217,12 @@ def test_sin_rewrite():
    assert sin(x).rewrite(cos) == cos(x - pi / 2, evaluate=False)
    assert sin(x).rewrite(sec) == 1 / sec(x - pi / 2, evaluate=False)
    assert sin(cos(x)).rewrite(Pow) == sin(cos(x))
+    assert sin(x).rewrite(besselj).subs(x,pi/4).n() == sin(x).subs(x,pi/4).n()


Can you use standard PEP 8 whitespace rules here and throughout the diff? This means one space after a comma, like

Suggested change

assert sin(x).rewrite(besselj).subs(x,pi/4).n() == sin(x).subs(x,pi/4).n()

assert sin(x).rewrite(besselj).subs(x, pi/4).n() == sin(x).subs(x, pi/4).n()

Ishanned added 2 commits December 11, 2022 19:35

rewrites for trigonometric functions as besselj and gamma

202e3f0

removed gamma

0130356

Ishanned force-pushed the rewrite-2 branch from b7f2fde to 0130356 Compare December 15, 2022 09:21

update

2f4109f

oscarbenjamin added the functions label Jan 3, 2023

github-actions bot added the Merge conflict label Jan 21, 2023

asmeurer reviewed Apr 25, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added rewrite for trigonometric functions as Besselj #24387

added rewrite for trigonometric functions as Besselj #24387

Ishanned commented Dec 14, 2022

sympy-bot commented Dec 14, 2022

github-actions bot commented Dec 15, 2022 •

edited

Ishanned commented Dec 15, 2022

oscarbenjamin commented Dec 15, 2022

Ishanned commented Dec 16, 2022

oscarbenjamin commented Jan 14, 2023

asmeurer Apr 25, 2023

asmeurer Apr 25, 2023

asmeurer Apr 25, 2023

asmeurer commented Apr 25, 2023

asmeurer Apr 25, 2023

	assert sin(x).rewrite(besselj).subs(x,pi/4).n() == sin(x).subs(x,pi/4).n()
	assert sin(x).rewrite(besselj).subs(x, pi/4).n() == sin(x).subs(x, pi/4).n()

added rewrite for trigonometric functions as Besselj #24387

Are you sure you want to change the base?

added rewrite for trigonometric functions as Besselj #24387

Conversation

Ishanned commented Dec 14, 2022

References to other Issues or PRs

Brief description of what is fixed or changed

Other comments

Release Notes

sympy-bot commented Dec 14, 2022

github-actions bot commented Dec 15, 2022 • edited

Ishanned commented Dec 15, 2022

oscarbenjamin commented Dec 15, 2022

Ishanned commented Dec 16, 2022

oscarbenjamin commented Jan 14, 2023

asmeurer Apr 25, 2023

Choose a reason for hiding this comment

asmeurer Apr 25, 2023

Choose a reason for hiding this comment

asmeurer Apr 25, 2023

Choose a reason for hiding this comment

asmeurer commented Apr 25, 2023

asmeurer Apr 25, 2023

Choose a reason for hiding this comment

github-actions bot commented Dec 15, 2022 •

edited