-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Changes for _eval_nseries in exp class for wrong outputs #25943
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
Conversation
✅ 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.
Update The release notes on the wiki have been updated. |
2cd390d
to
05cced9
Compare
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 [f6bf7438] | Ratio | Benchmark (Parameter) |
|----------|----------------------|---------------------|---------|----------------------------------------------------------------------|
| - | 69.5±1ms | 44.5±0.4ms | 0.64 | integrate.TimeIntegrationRisch02.time_doit(10) |
| - | 69.0±0.9ms | 43.0±0.4ms | 0.62 | integrate.TimeIntegrationRisch02.time_doit_risch(10) |
| + | 18.9±0.5μs | 29.9±0.6μs | 1.58 | integrate.TimeIntegrationRisch03.time_doit(1) |
| - | 5.55±0.08ms | 2.85±0.01ms | 0.51 | logic.LogicSuite.time_load_file |
| - | 73.0±0.7ms | 28.7±0.1ms | 0.39 | polys.TimeGCD_GaussInt.time_op(1, 'dense') |
| - | 26.2±0.1ms | 17.0±0.06ms | 0.65 | polys.TimeGCD_GaussInt.time_op(1, 'expr') |
| - | 73.8±0.3ms | 29.0±0.03ms | 0.39 | polys.TimeGCD_GaussInt.time_op(1, 'sparse') |
| - | 254±0.8ms | 125±0.5ms | 0.49 | polys.TimeGCD_GaussInt.time_op(2, 'dense') |
| - | 257±2ms | 125±0.5ms | 0.49 | polys.TimeGCD_GaussInt.time_op(2, 'sparse') |
| - | 659±2ms | 373±2ms | 0.57 | polys.TimeGCD_GaussInt.time_op(3, 'dense') |
| - | 661±3ms | 372±1ms | 0.56 | polys.TimeGCD_GaussInt.time_op(3, 'sparse') |
| - | 495±2μs | 286±0.8μs | 0.58 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(1, 'dense') |
| - | 1.79±0.02ms | 1.05±0.01ms | 0.59 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(2, 'dense') |
| - | 5.78±0.05ms | 3.05±0.01ms | 0.53 | polys.TimeGCD_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 443±2μs | 227±1μs | 0.51 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(1, 'dense') |
| - | 1.48±0.01ms | 660±6μs | 0.45 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(2, 'dense') |
| - | 4.79±0.03ms | 1.64±0.01ms | 0.34 | polys.TimeGCD_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 376±4μs | 204±0.9μs | 0.54 | polys.TimeGCD_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 2.46±0ms | 1.24±0ms | 0.5 | polys.TimeGCD_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 10.3±0.03ms | 4.29±0.02ms | 0.42 | polys.TimeGCD_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 361±2μs | 171±1μs | 0.47 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(1, 'dense') |
| - | 2.51±0.04ms | 914±5μs | 0.36 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 9.57±0.1ms | 2.62±0.01ms | 0.27 | polys.TimeGCD_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 1.03±0.02ms | 427±5μs | 0.41 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 1.74±0.01ms | 501±3μs | 0.29 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.89±0.1ms | 1.81±0.01ms | 0.31 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'dense') |
| - | 8.54±0.02ms | 1.48±0.01ms | 0.17 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse') |
| - | 286±1μs | 64.5±0.3μs | 0.23 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 3.43±0.02ms | 395±4μs | 0.12 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 3.97±0.04ms | 276±0.9μs | 0.07 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 7.05±0.03ms | 1.28±0.01ms | 0.18 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'dense') |
| - | 8.57±0.04ms | 834±4μs | 0.1 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse') |
| - | 5.03±0.03ms | 2.96±0.01ms | 0.59 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| - | 12.0±0.07ms | 6.51±0.03ms | 0.54 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'dense') |
| - | 22.3±0.06ms | 9.03±0.04ms | 0.4 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| - | 5.22±0.03ms | 868±3μs | 0.17 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse') |
| - | 12.8±0.08ms | 6.99±0.02ms | 0.55 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse') |
| - | 102±3ms | 25.7±0.1ms | 0.25 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'dense') |
| - | 166±0.9ms | 53.7±0.5ms | 0.32 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse') |
| - | 172±0.5μs | 111±0.3μs | 0.64 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'dense') |
| - | 360±4μs | 216±4μs | 0.6 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(1, 'sparse') |
| - | 4.28±0.04ms | 834±4μs | 0.19 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'dense') |
| - | 5.31±0.07ms | 375±0.5μs | 0.07 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse') |
| - | 19.8±0.2ms | 2.83±0.04ms | 0.14 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'dense') |
| - | 22.9±0.08ms | 619±2μs | 0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse') |
| - | 482±4μs | 135±0.7μs | 0.28 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| - | 4.69±0.05ms | 610±2μs | 0.13 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'dense') |
| - | 5.26±0.05ms | 137±0.9μs | 0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| - | 13.3±0.1ms | 1.30±0ms | 0.1 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'dense') |
| - | 14.0±0.04ms | 142±1μs | 0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| - | 132±1μs | 76.7±0.3μs | 0.58 | solve.TimeMatrixOperations.time_rref(3, 0) |
| - | 251±1μs | 88.8±0.7μs | 0.35 | solve.TimeMatrixOperations.time_rref(4, 0) |
| - | 24.2±0.03ms | 10.2±0.03ms | 0.42 | solve.TimeSolveLinSys189x49.time_solve_lin_sys |
Full benchmark results can be found as artifacts in GitHub Actions |
In absence of any other feedback, this looks like it is restoring the desired behavior and can be committed. |
Please refrain from merging this PR. I introduced these changes around 1.5 years back and also worked on them during my GSoC project. I would like to thoroughly review these changes before it goes in. (I'll look into it by the weekend) |
Thanks for your work. Once you address this, I'll point you to a couple more issues that we should be fixing. |
The changes look good to me but I am not sure why does 1 test fail. It seems unrelated though. @oscarbenjamin could you help us out here ? |
cc @arnabnandikgp could you also squash these commits into 1 or 2 .Thanks |
dde83d5
to
a134a43
Compare
Signed-off-by: arnabnandikgp <arnabnandi2002@gmail.com>
a134a43
to
6d80013
Compare
Signed-off-by: arnabnandikgp <arnabnandi2002@gmail.com>
References to other Issues or PRs
Fixes #24266
Brief description of what is fixed or changed
Added conditions to handle the case of imaginary exponents along with their test cases
Other comments
Changed code to accommodate cases for when exponential class is called for exponential as well as non exponential bases with imaginary powers and categorised tests as follows:
Type 1: exp(f(x))
Type 2: c**f(x)
Type 3: f(y)**g(x)
Release Notes