-
-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix latex printing of undefined functions raised to a power #26331
base: master
Are you sure you want to change the base?
Fix latex printing of undefined functions raised to a power #26331
Conversation
The used notation to print functions raised to a power is standardized across all types of functions, the recognized and not recognized ones. The undefined functions raised to power should be wrapped between parenthesis. Also, corresponding tests have been modified.
✅ Hi, I am the SymPy bot. 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:
This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.13. Click here to see the pull request description that was parsed.
|
Benchmark results from GitHub Actions Lower numbers are good, higher numbers are bad. A ratio less than 1 Significantly changed benchmark results (PR vs master) Significantly changed benchmark results (master vs previous release) | Change | Before [a00718ba] | After [14a6e27d] | Ratio | Benchmark (Parameter) |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| - | 67.8±0.8ms | 44.5±0.6ms | 0.66 | integrate.TimeIntegrationRisch02.time_doit(10) |
| - | 68.6±0.6ms | 43.6±0.2ms | 0.63 | integrate.TimeIntegrationRisch02.time_doit_risch(10) |
| + | 18.3±0.2μs | 30.4±0.2μs | 1.67 | integrate.TimeIntegrationRisch03.time_doit(1) |
| - | 5.35±0.03ms | 2.86±0.01ms | 0.53 | logic.LogicSuite.time_load_file |
| - | 74.6±0.2ms | 29.3±0.07ms | 0.39 | polys.TimeGCD_GaussInt.time_op(1, 'dense') |
| - | 73.6±0.2ms | 29.3±0.1ms | 0.4 | polys.TimeGCD_GaussInt.time_op(1, 'sparse') |
| - | 254±0.8ms | 128±1ms | 0.5 | polys.TimeGCD_GaussInt.time_op(2, 'dense') |
| - | 257±2ms | 128±0.6ms | 0.5 | polys.TimeGCD_GaussInt.time_op(2, 'sparse') |
| - | 657±7ms | 374±1ms | 0.57 | polys.TimeGCD_GaussInt.time_op(3, 'dense') |
| - | 657±5ms | 378±2ms | 0.57 | polys.TimeGCD_GaussInt.time_op(3, 'sparse') |
| - | 494±7μs | 287±0.7μs | 0.58 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense') |
| - | 1.80±0.02ms | 1.07±0ms | 0.59 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense') |
| - | 5.88±0.08ms | 3.16±0.02ms | 0.54 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 453±2μs | 229±2μs | 0.51 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense') |
| - | 1.48±0.01ms | 673±2μs | 0.45 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense') |
| - | 4.98±0.02ms | 1.64±0.01ms | 0.33 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 387±2μs | 205±1μs | 0.53 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 2.44±0.01ms | 1.25±0.01ms | 0.51 | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 9.99±0.04ms | 4.49±0.03ms | 0.45 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 358±1μs | 168±0.7μs | 0.47 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense') |
| - | 2.47±0.01ms | 919±20μs | 0.37 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 9.60±0.07ms | 2.69±0.01ms | 0.28 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 1.05±0ms | 430±3μs | 0.41 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 1.72±0.01ms | 505±3μs | 0.29 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.92±0.06ms | 1.81±0.02ms | 0.31 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense') |
| - | 8.54±0.02ms | 1.49±0.01ms | 0.17 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse') |
| - | 287±1μs | 64.8±0.5μs | 0.23 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 3.47±0.04ms | 406±9μs | 0.12 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 4.01±0.03ms | 279±0.7μs | 0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 7.05±0.7ms | 1.27±0.01ms | 0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense') |
| - | 8.73±0.07ms | 838±5μs | 0.1 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse') |
| - | 5.05±0ms | 2.99±0.01ms | 0.59 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| - | 12.2±0.1ms | 6.65±0.04ms | 0.54 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 22.5±0.1ms | 9.06±0.05ms | 0.4 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.27±0.01ms | 875±4μs | 0.17 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 12.6±0.05ms | 7.04±0.02ms | 0.56 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse') |
| - | 103±0.4ms | 26.3±0.1ms | 0.26 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 167±0.6ms | 53.9±0.09ms | 0.32 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 173±1μs | 111±0.7μs | 0.64 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 362±1μs | 216±0.5μs | 0.6 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse') |
| - | 4.24±0.04ms | 849±10μs | 0.2 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 5.37±0.03ms | 385±5μs | 0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse') |
| - | 19.6±0.1ms | 2.83±0.01ms | 0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 22.9±0.1ms | 627±2μs | 0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse') |
| - | 587±100μs | 136±2μs | 0.23 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| - | 4.71±0.04ms | 628±10μs | 0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 5.40±0.03ms | 138±0.8μs | 0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| - | 13.1±0.06ms | 1.29±0.01ms | 0.1 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 14.0±0.1ms | 141±0.7μs | 0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| - | 134±1μs | 74.8±0.5μs | 0.56 | solve.TimeMatrixOperations.time_rref(3, 0) |
| - | 254±2μs | 87.6±0.4μs | 0.35 | solve.TimeMatrixOperations.time_rref(4, 0) |
| - | 24.2±0.1ms | 10.0±0.03ms | 0.41 | solve.TimeSolveLinSys189x49.time_solve_lin_sys |
| - | 28.2±0.09ms | 15.4±0.05ms | 0.55 | solve.TimeSparseSystem.time_linsolve_Aaug(20) |
| - | 54.8±0.2ms | 24.9±0.08ms | 0.45 | solve.TimeSparseSystem.time_linsolve_Aaug(30) |
| - | 28.1±0.2ms | 15.2±0.08ms | 0.54 | solve.TimeSparseSystem.time_linsolve_Ab(20) |
| - | 54.6±0.2ms | 24.6±0.1ms | 0.45 | solve.TimeSparseSystem.time_linsolve_Ab(30) |
Full benchmark results can be found as artifacts in GitHub Actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is accepted_latex_functions
an appropriate condition for this?
Lines 36 to 44 in 07da3ce
# Hand-picked functions which can be used directly in both LaTeX and MathJax | |
# Complete list at | |
# https://docs.mathjax.org/en/latest/tex.html#supported-latex-commands | |
# This variable only contains those functions which SymPy uses. | |
accepted_latex_functions = ['arcsin', 'arccos', 'arctan', 'sin', 'cos', 'tan', | |
'sinh', 'cosh', 'tanh', 'sqrt', 'ln', 'log', 'sec', | |
'csc', 'cot', 'coth', 're', 'im', 'frac', 'root', | |
'arg', | |
] |
@haru-44 I don't understand what do you mean with this, can you elaborate more ? |
I understand that the functions in
|
I guess that makes sense, I will try to fix it. |
assert latex(Ci(x)**2) == r'\operatorname{Ci}^{2}{\left(x \right)}' | ||
assert latex(Shi(x)**2) == r'\left(\operatorname{Shi} {\left(x \right)}\right)^{2}' | ||
assert latex(Si(x)**2) == r'\left(\operatorname{Si} {\left(x \right)}\right)^{2}' | ||
assert latex(Ci(x)**2) == r'\left(\operatorname{Ci} {\left(x \right)}\right)^{2}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does these changes while Chi
does not? I just notice the problem of asymmetry here. I think that maybe only Function
should be parenthesized as per design, not other pre defined SymPy functions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@haru-44 pointed out that we should define a dictionary with some defined functions, not just the ones defined in accepted_latex_functions
, to solve such issue.
The used notation to print functions raised to a power is standardized across all types of functions, the recognized and not recognized ones. The undefined functions raised to power should be wrapped between parenthesis. Also, corresponding tests have been modified.
References to other Issues or PRs
Fixes #24596
Brief description of what is fixed or changed
The used notation to print functions raised to a power is standardized across
all types of functions, the recognized and not recognized ones. The undefined
functions raised to power should be wrapped between parenthesis.
So instead of
func**2(x)
it should be(func(x))**2
but for regular functions like trigonometric functions, they stay the same
sin**2(x)
Before the patch
After the patch
Also, corresponding tests have been modified.
Other comments
Release Notes