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

Fix for #25293 and better treatment of Heaviside(t) #25633

Merged
merged 2 commits into from
Sep 6, 2023

Conversation

hanspi42
Copy link
Contributor

@hanspi42 hanspi42 commented Sep 6, 2023

References to other Issues or PRs

Fixes #25293

Brief description of what is fixed or changed

Time expressions coming from system theory may contain a product of DiracDelta functions with sinc functions. This gave nan as a result. Now the Laplace transform will try to rewrite it as sinc, but also, if the result would be nan in conjunction with DiracDelta, not return nan, but None (i.e., say that it cannot evaluate).

A second change is that _laplace_transform will now remove incoming Heaviside(t) factors, which reduces the number of calls to integrate, but otherwise does not change anything. This second part is not mentioned in the release notes.

Other comments

Release Notes

  • integrals
    • The Laplace transform can now deal better with expressions that contain products of sin(t)/t and DiracDelta(t) with arbitrary time shifts.

@sympy-bot
Copy link

sympy-bot commented Sep 6, 2023

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:

  • integrals
    • The Laplace transform can now deal better with expressions that contain products of sin(t)/t and DiracDelta(t) with arbitrary time shifts. (#25633 by @hanspi42)

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.
#### References to other Issues or PRs
Fixes #25293


#### Brief description of what is fixed or changed
Time expressions coming from system theory may contain a product of `DiracDelta` functions with `sinc` functions. This gave `nan` as a result. Now the Laplace transform will try to rewrite it as `sinc`, but also, if the result would be `nan` in conjunction with `DiracDelta`, not return `nan`, but `None` (i.e., say that it cannot evaluate).

A second change is that `_laplace_transform` will now remove incoming `Heaviside(t)` factors, which reduces the number of calls to `integrate`, but otherwise does not change anything. This second part is not mentioned in the release notes.

#### Other comments


#### Release Notes

<!-- BEGIN RELEASE NOTES -->
* integrals
  * The Laplace transform can now deal better with expressions that contain products of `sin(t)/t` and `DiracDelta(t)` with arbitrary time shifts. 
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

@hanspi42
Copy link
Contributor Author

hanspi42 commented Sep 6, 2023

@oscarbenjamin: A fix of #25633; not a big thing, so if you are mentally into laplace.py right now anyway, I'd be happy if you could look at it.

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

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)

| Change   | Before [8059df73] <sympy-1.12^0>   | After [4b65d90f]    |   Ratio | Benchmark (Parameter)                                                |
|----------|------------------------------------|---------------------|---------|----------------------------------------------------------------------|
| -        | 107±3ms                            | 67.6±0.4ms          |    0.63 | integrate.TimeIntegrationRisch02.time_doit(10)                       |
| -        | 104±2ms                            | 66.6±0.3ms          |    0.64 | integrate.TimeIntegrationRisch02.time_doit_risch(10)                 |
| +        | 25.8±0.2μs                         | 46.4±0.2μs          |    1.8  | integrate.TimeIntegrationRisch03.time_doit(1)                        |
| -        | 8.34±0.03ms                        | 4.47±0.03ms         |    0.54 | logic.LogicSuite.time_load_file                                      |
| -        | 2.46±0ms                           | 787±3μs             |    0.32 | polys.TimePREM_LinearDenseQuadraticGCD.time_op(3, 'sparse')          |
| -        | 12.0±0.02ms                        | 2.34±0ms            |    0.2  | polys.TimePREM_LinearDenseQuadraticGCD.time_op(5, 'sparse')          |
| -        | 443±10μs                           | 98.1±0.3μs          |    0.22 | polys.TimePREM_QuadraticNonMonicGCD.time_op(1, 'sparse')             |
| -        | 5.60±0.09ms                        | 432±2μs             |    0.08 | polys.TimePREM_QuadraticNonMonicGCD.time_op(3, 'sparse')             |
| -        | 12.1±0.02ms                        | 1.31±0ms            |    0.11 | polys.TimePREM_QuadraticNonMonicGCD.time_op(5, 'sparse')             |
| -        | 7.40±0.03ms                        | 4.83±0.3ms          |    0.65 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(2, 'sparse') |
| -        | 31.9±0.02ms                        | 14.5±0.04ms         |    0.45 | polys.TimeSUBRESULTANTS_LinearDenseQuadraticGCD.time_op(3, 'sparse') |
| -        | 8.35±0.03ms                        | 1.41±0ms            |    0.17 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(1, 'sparse')    |
| -        | 19.3±0.06ms                        | 11.1±0.03ms         |    0.57 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(2, 'sparse')    |
| -        | 245±0.4ms                          | 85.0±0.3ms          |    0.35 | polys.TimeSUBRESULTANTS_QuadraticNonMonicGCD.time_op(3, 'sparse')    |
| -        | 7.36±0.04ms                        | 635±3μs             |    0.09 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(3, 'sparse')     |
| -        | 31.5±0.1ms                         | 1.02±0ms            |    0.03 | polys.TimeSUBRESULTANTS_SparseGCDHighDegree.time_op(5, 'sparse')     |
| -        | 740±20μs                           | 242±3μs             |    0.33 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(1, 'sparse') |
| -        | 7.39±0.02ms                        | 237±1μs             |    0.03 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(3, 'sparse') |
| -        | 19.5±0.2ms                         | 245±0.8μs           |    0.01 | polys.TimeSUBRESULTANTS_SparseNonMonicQuadratic.time_op(5, 'sparse') |
| -        | 196±6μs                            | 105±0.4μs           |    0.54 | solve.TimeMatrixOperations.time_rref(3, 0)                           |
| -        | 370±5μs                            | 129±1μs             |    0.35 | solve.TimeMatrixOperations.time_rref(4, 0)                           |
| -        | 37.8±0.08ms                        | 16.2±0.06ms         |    0.43 | solve.TimeSolveLinSys189x49.time_solve_lin_sys                       |

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

@oscarbenjamin
Copy link
Collaborator

Looks good!

@oscarbenjamin oscarbenjamin merged commit 1df6819 into sympy:master Sep 6, 2023
55 checks passed
@hanspi42 hanspi42 deleted the heaviside_dirac branch September 7, 2023 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

laplace_transform returns nan
3 participants