Implement lazy dictionary for trigonometric special angles

[sylee957]

References to other Issues or PRs

#24172

Brief description of what is fixed or changed

I've implemented lazy dictionary based on functional approach
It should be more easier to implement, and have more robust behavior than object-oriented approach in the original suggestion.

This can solve some hidden slowdown issues with trig rewrites like cos(x + pi/6).expand(trig=True)
I'm trying to collect these issues though.

I'm also planning to make trigonometric_special.py to make the special angle formula more explicitly accessible for users, than having to access silently via rewrite.
It could be more easier to use for users who want more concrete input structure like cos_special(1, 17), cos_special(2, 17), ....

Other comments

Release Notes

• functions
  • Fixed slowdown issues of initial run caused by evaluating cos(pi/257) eagerly even if it is not needed. Now, cos(x + pi/6).expand(trig=True) gives more faster result.

[sympy-bot]

✅

Hi, I am the SymPy bot (v169). 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
  • Fixed slowdown issues of initial run caused by evaluating cos(pi/257) eagerly even if it is not needed. Now, cos(x + pi/6).expand(trig=True) gives more faster result. (#24566 by @sylee957)

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. -->

#24172

#### Brief description of what is fixed or changed

I've implemented lazy dictionary based on functional approach
It should be more easier to implement, and have more robust behavior than object-oriented approach in the original suggestion.

This can solve some hidden slowdown issues with trig rewrites like `cos(x + pi/6).expand(trig=True)`
I'm trying to collect these issues though.

I'm also planning to make `trigonometric_special.py` to make the special angle formula more explicitly accessible for users, than having to access silently via `rewrite`.
It could be more easier to use for users who want more concrete input structure like `cos_special(1, 17), cos_special(2, 17), ...`.

#### 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. Formerly, `log(-x)` incorrectly gave `-log(x)`.

* physics.units
  * Corrected a semantical error in the conversion between volt and statvolt which
    reported the volt as being larger than the statvolt.

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
  - Fixed slowdown issues of initial run caused by evaluating `cos(pi/257)` eagerly even if it is not needed. Now, `cos(x + pi/6).expand(trig=True)` gives more faster result.
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

[sympy-bot]

🟠

Hi, I am the SymPy bot (v169). I've noticed that some of your commits add or delete files. Since this is sometimes done unintentionally, I wanted to alert you about it.

This is an experimental feature of SymPy Bot. If you have any feedback on it, please comment at sympy/sympy-bot#75.

The following commits add new files:

• ac99279:
  • sympy/functions/elementary/trigonometric_special.py
• 4b44557:
  • sympy/functions/elementary/_trigonometric_special.py

The following commits delete files:

• 4b44557:
  • sympy/functions/elementary/trigonometric_special.py

If these files were added/deleted on purpose, you can ignore this message.

[github-actions]

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]       [c25ecf0e]
     <sympy-1.11.1^0>                 
-         964±3μs          604±1μs     0.63  solve.TimeSparseSystem.time_linear_eq_to_matrix(10)
-        2.77±0ms         1.14±0ms     0.41  solve.TimeSparseSystem.time_linear_eq_to_matrix(20)
-     5.59±0.01ms      1.68±0.01ms     0.30  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).

[smichr]

Looks ok to me. Maybe someone else will have suggestions.

[asmeurer]

Isn't a function already a "lazy dictionary"? I don't see the need for the indirection here. You can just have cst_table(n) which returns the result for n = 3, 5, 7, 257 (and that caches the value 257). That would change the code that uses it from cst_table[n] to cst_table(n), but it's all internal code so that doesn't matter.

[sylee957]

I would just want to keep dictionary than functions.

Because having functions implies something more, like making the logic sequential and and hierarchical than the mapping,
which complicates the problems with more questions like "should 5 be checked first than 3?", or such

if x == 3:
    ...
elif x == 5:
    ...

[asmeurer]

Because having functions implies something more, like making the logic sequential and and hierarchical than the mapping,
which complicates the problems with more questions like "should 5 be checked first than 3?", or such

This reasoning really doesn't make any sense to me. An if chain doesn't imply any of those things, and I don't see why anyone reading the code would think that. Are you really saying that if/elif chains shouldn't be used if the conditions are mutually exclusive?

The code here is unnecessarily complicated with indirections.

if pi_coeff.q in cst_table_some:
    rv = chebyshevt(pi_coeff.p, cst_table_some[pi_coeff.q]())

could just be

fermat_primes = [3, 5, 17, 257] # defined above somewhere

if pi_coeff.q in fermat_primes:
    rv = chebyshevt(pi_coeff.p, cos_table(pi_coeff.q))

where cos_table is just a single cached functions that computes the various values.

[asmeurer]

By the way, it might be worth sympifying a string form of cos_257. It's a bit of a hack, but in my tests it's considerably faster. If we can speed up the actual construction that would be great, but I expect that would require a lot of cleaning up of assumptions use in constructors which will be harder due how it breaks things.

[asmeurer]

By the way, it might be worth sympifying a string form of cos_257. It's a bit of a hack, but in my tests it's considerably faster. If we can speed up the actual construction that would be great, but I expect that would require a lot of cleaning up of assumptions use in constructors which will be harder due how it breaks things.

I should note that that's specifically with sympify(evaluate=False) (although it will appear to be fast without evaluate=False if you've already computed the expression because of the cache).

[sylee957]

The code here is unnecessarily complicated with indirections.

It doesn't necessarily characterize the reason to use functions for me,
The only necessary reason to use function is when there are infinitely many stuff, such that it is the only option, but there are more options unless things reveal in the future how this should generalize.
We just completed the cos table, which is too trivial to overview all about these,
so I'll let anyone else to change that if they don't like it.

By the way, it might be worth sympifying a string form of cos_257. It's a bit of a hack, but in my tests it's considerably faster.

I'm not sure how you've done that, without an obvious nasty workaround of keeping the form of str(cos_257()) somewhere and using that.
but I'm not sure if there is procedural capabiity in sympify. (like sympify(r'x = 3; y = x ** 2; y') to give 9?)
I'm aware of some other workaround to build the huge strings procedurally, like storing some rules of strings instead of the string, and substituting strings by the rules.