Opened 15 months ago
Closed 8 months ago
#24212 closed defect (fixed)
Fresnel integrals
Reported by: | rws | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.2 |
Component: | symbolics | Keywords: | |
Cc: | mforets | Merged in: | |
Authors: | Marcelo Forets | Reviewers: | Ralf Stephan |
Report Upstream: | N/A | Work issues: | |
Branch: | f49ce18 (Commits) | Commit: | f49ce18fd615bb4aad046006d57c5054e43b353e |
Dependencies: | #24425 | Stopgaps: |
Description (last modified by )
The SymPy-Sage interface raises an error because there are no Fresnel integral functions in Sage:
sage: integrate(sin(x^2), x, algorithm='sympy') ... AttributeError: see: sage: import sympy sage: x = sympy.Symbol('x') sage: sympy.integrate(sympy.sin(x^2), x) 3*sqrt(2)*sqrt(pi)*fresnels(sqrt(2)*x/sqrt(pi))*gamma(3/4)/(8*gamma(7/4))
They are holonomic so they should be in Sage.
Change History (55)
comment:1 Changed 15 months ago by
- Description modified (diff)
comment:2 Changed 15 months ago by
- Branch set to u/mforets/fresnel_integrals
- Cc mforets added
- Commit set to e52fb2eec054b050f2d1c578ef3e7bd647fa407e
comment:3 Changed 15 months ago by
Oh, and the beta(...
--> beta...
is because the missing parentheses were confusing my text editor, so that the entire file from that line on was shown as comments. I should remember to undo this change :)
comment:4 Changed 15 months ago by
Returning None
(or just passing) in _eval_
amounts to returning a held expression. _evalf_
is only used for inexact arguments or .n()
:
sage: fresnel_sin(0.) 0.000000000000000 sage: fresnel_sin(0).n() 0.000000000000000
For results to exact or symbolic arguments you need to implement them in _eval_
.
comment:5 Changed 15 months ago by
You would just need to handle 0
and oo
in _eval_
, and handle oddness before it, i.e. if arg.is_negative(): return -fresnel_sin(-arg)
.
comment:6 Changed 15 months ago by
- Commit changed from e52fb2eec054b050f2d1c578ef3e7bd647fa407e to f158660589867242a065c8995b16087755793d66
Branch pushed to git repo; I updated commit sha1. New commits:
f158660 | symbolics in fresnel_sin eval
|
comment:7 Changed 15 months ago by
ok, that fixed the 0, oo and -oo evaluations. i added two new tests for the I*oo
cases, but this is still wrong. since
sage: SR(I*oo).is_infinity() True
probably i could use x.is_real()
too.
comment:8 Changed 15 months ago by
You mean unsigned_infinity
?
comment:9 follow-up: ↓ 10 Changed 15 months ago by
i should relate unsigned_infinity
to test if the argument is complex and infinite? the doc of unsigned_infinity
is precarious:
sage: unsigned_infinity? Type: UnsignedInfinity String form: Infinity File: ~/Tools/sage-master/local/lib/python2.7/site-packages/sage/rings/infinity.py Docstring: Initialize "self". Init docstring: Initialize "self".
i'm tempted to add a ticket to explain this object more.
comment:10 in reply to: ↑ 9 Changed 15 months ago by
Replying to mforets:
i should relate
unsigned_infinity
to test if the argument is complex and infinite?
No, I was not sure what you had in mind. If you want to test if the argument is complex and infinite then do: if arg.is_infinity() and not arg.is_real()
. Technically a False for arg.is_real()
just means "I don't know" so you get True
for SR(unsigned_infinity).is_real()
, but the correct behaviour must wait until we have is_complex()
. Just use not arg.is_real()
for now.
comment:11 Changed 15 months ago by
- Commit changed from f158660589867242a065c8995b16087755793d66 to ada47e4c62449f2c32c86ae0cb8fe8c9b5b23e7c
Branch pushed to git repo; I updated commit sha1. New commits:
ada47e4 | handle complex infinities
|
comment:12 Changed 15 months ago by
- Commit changed from ada47e4c62449f2c32c86ae0cb8fe8c9b5b23e7c to 869020b95344d81e5e4ff27f6f5829614ac588b5
Branch pushed to git repo; I updated commit sha1. New commits:
869020b | add cosine Fresnel integral
|
comment:13 Changed 15 months ago by
- Status changed from new to needs_review
comment:14 Changed 15 months ago by
oh, there's no conversion back to sage:
sage: fresnel_sin(x)._sympy_() # ok fresnels(x) sage: fresnel_sin(x)._sympy_()._sage_() # not working!
comment:15 Changed 15 months ago by
- Commit changed from 869020b95344d81e5e4ff27f6f5829614ac588b5 to e930137c2c2de67ad5f37b347ca79fd743ad484d
Branch pushed to git repo; I updated commit sha1. New commits:
e930137 | add sympy -> sage conversion, and doctest
|
comment:16 Changed 15 months ago by
See how easy it is to add conversions now.
comment:17 Changed 15 months ago by
See how easy it is to add conversions now.
awesome! it took me what, 15 minutes? and it was the first time i was doing that.
comment:18 Changed 15 months ago by
- Status changed from needs_review to needs_work
Patchbot has relevant doctest errors.
comment:19 Changed 15 months ago by
- Commit changed from e930137c2c2de67ad5f37b347ca79fd743ad484d to 2a0e907c8ca56d8a0a267b5ef39e8da30dc6b999
Branch pushed to git repo; I updated commit sha1. New commits:
2a0e907 | fix laplace test
|
comment:20 Changed 15 months ago by
- Status changed from needs_work to needs_review
Indeed, thanks for noticing it!
Some patchbots reported an error RuntimeError: cases argument not a sequence
happening with some random tests. This seems to me unrelated to this ticket, but maybe i'm wrong.
comment:21 Changed 15 months ago by
Indeed, the cases fail has nothing to do with this ticket. It's a bug that is rare and happens randomly (#24284).
comment:22 Changed 15 months ago by
- Status changed from needs_review to needs_work
Docs are looking good. The patchbot errors are unrelated so I would be positive. However, the derivative is missing and there is a typo in the docs: write \operatorname{C}
not \operatorname(C)
in the formula. Could you please add that?
comment:23 Changed 15 months ago by
- Commit changed from 2a0e907c8ca56d8a0a267b5ef39e8da30dc6b999 to e6b884e36444490289062a47b1d8fb52b65cf63c
Branch pushed to git repo; I updated commit sha1. New commits:
e6b884e | fix operatorname and add derivatives
|
comment:24 Changed 15 months ago by
- Status changed from needs_work to needs_review
comment:25 Changed 15 months ago by
- Reviewers set to Ralf Stephan
- Status changed from needs_review to positive_review
Thanks for your work.
comment:26 Changed 15 months ago by
well, thanks for the review and all your help.
comment:27 Changed 15 months ago by
- Status changed from positive_review to needs_work
Merge conflict
comment:28 Changed 14 months ago by
- Branch changed from u/mforets/fresnel_integrals to u/rws/fresnel_integrals
comment:29 Changed 14 months ago by
- Commit changed from e6b884e36444490289062a47b1d8fb52b65cf63c to 44c2bdb64057ff101f7332171ba085b57ee41734
- Status changed from needs_work to positive_review
I merged with 8.2.beta0 + #24171 to avoid another conflict.
New commits:
669ea22 | 24171: Formal set membership function
|
34c9773 | 24171: make RealSet inherit from Set_generic
|
af82717 | 24171: remove RealSet import; SR set coercion doctests
|
592ce87 | 24171: Py3 fixes
|
aa11a3e | Merge branch 'develop' into t/24171/formal_set_membership_function
|
44c2bdb | Merge branch 'u/rws/formal_set_membership_function' of git://trac.sagemath.org/sage into t/24212/fresnel_integrals
|
comment:30 Changed 14 months ago by
well done, thanks.
comment:32 Changed 14 months ago by
The depended-on #24171 needs a patch so I'll squash it all, keeping authorship info correct.
comment:33 Changed 14 months ago by
comment:34 Changed 14 months ago by
- Dependencies set to #24284 #24378
comment:35 Changed 14 months ago by
- Branch changed from u/rws/fresnel_integrals to u/rws/24212
comment:36 Changed 14 months ago by
- Branch changed from u/rws/24212 to u/rws/24212-1
comment:37 Changed 14 months ago by
- Branch changed from u/rws/24212-1 to u/rws/24212-2
comment:38 Changed 14 months ago by
- Commit changed from 44c2bdb64057ff101f7332171ba085b57ee41734 to e3cd31afa599b019dfe2c3312844f20644d981d8
- Dependencies #24284 #24378 deleted
- Status changed from needs_work to positive_review
I moved the code into error.py
as the Fresnel integrals are related to error functions. This also makes it independent from other tickets.
New commits:
e3cd31a | 24212: Fresnel integrals
|
comment:39 Changed 14 months ago by
- Status changed from positive_review to needs_work
File "src/sage/symbolic/random_tests.py", line 265, in sage.symbolic.random_tests.? Failed example: random_expr(50, nvars=3, coeff_generator=CDF.random_element) # random Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 517, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 920, in compile_and_execute exec(compiled, globs) File "<doctest sage.symbolic.random_tests.?[2]>", line 1, in <module> random_expr(Integer(50), nvars=Integer(3), coeff_generator=CDF.random_element) # random File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 288, in random_expr return random_expr_helper(size, internal, leaves, verbose) File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 232, in random_expr_helper children = [random_expr_helper(n+1, internal, leaves, verbose) for n in nodes_per_child] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 232, in random_expr_helper children = [random_expr_helper(n+1, internal, leaves, verbose) for n in nodes_per_child] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 232, in random_expr_helper children = [random_expr_helper(n+1, internal, leaves, verbose) for n in nodes_per_child] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 232, in random_expr_helper children = [random_expr_helper(n+1, internal, leaves, verbose) for n in nodes_per_child] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 232, in random_expr_helper children = [random_expr_helper(n+1, internal, leaves, verbose) for n in nodes_per_child] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 232, in random_expr_helper children = [random_expr_helper(n+1, internal, leaves, verbose) for n in nodes_per_child] File "/home/buildbot/slave/sage_git/build/local/lib/python2.7/site-packages/sage/symbolic/random_tests.py", line 235, in random_expr_helper return r[1](*children) File "sage/symbolic/expression.pyx", line 3537, in sage.symbolic.expression.Expression.__invert__ (build/cythonized/sage/symbolic/expression.cpp:25798) return 1/self File "sage/structure/element.pyx", line 1660, in sage.structure.element.Element.__div__ (build/cythonized/sage/structure/element.c:12835) return coercion_model.bin_op(left, right, div) File "sage/structure/coerce.pyx", line 1081, in sage.structure.coerce.CoercionModel_cache_maps.bin_op (build/cythonized/sage/structure/coerce.c:9515) return PyObject_CallObject(op, xy) File "sage/structure/element.pyx", line 1655, in sage.structure.element.Element.__div__ (build/cythonized/sage/structure/element.c:12758) return (<Element>left)._div_(right) File "sage/symbolic/expression.pyx", line 3518, in sage.symbolic.expression.Expression._div_ (build/cythonized/sage/symbolic/expression.cpp:25588) raise ZeroDivisionError("Symbolic division by zero") ZeroDivisionError: Symbolic division by zero ********************************************************************** 1 item had failures: 1 of 5 in sage.symbolic.random_tests.? [35 tests, 1 failure, 22.02 s] ---------------------------------------------------------------------- sage -t --long src/sage/symbolic/random_tests.py # 1 doctest failed
comment:40 Changed 14 months ago by
random_expr
is really interesting. It should be made easier to debug, and errors caught/not caught should be reviewed because TypeError
s from ECL represent real bugs and RuntimeError
s from Pynac should be rebranded in the Python layer. Also sometimes it takes really really long time. After that, someone should do a day-long run to catch any remaining problems.
comment:41 Changed 14 months ago by
The reason why the doctest fails is that, despite fixing the random seed, the functions and symbols picked by random_expr
are different. This, at some point in the buildup of the expression, leads to the sub-expression kronecker_delta(-0.6165326641917295 + 0.15117833554974136*I, e)
which immediately evaluates to 0
. The 0
progresses eventually into a denominator, causing the error. random_expr
is not written to avoid such errors if they happen, nor does it avoid other exceptions thrown. The author of the doctest apparently just chose a random seed that happens to not throw errors.
Of course the actual bug is that random_expr
changes behaviour when unrelated code is changed.
comment:42 Changed 14 months ago by
- Dependencies set to #24425
comment:43 Changed 14 months ago by
- Commit changed from e3cd31afa599b019dfe2c3312844f20644d981d8 to 96521a4ca9661dae2623c038517143c96f70e6c2
comment:44 Changed 14 months ago by
- Status changed from needs_work to needs_review
Dependency needs review.
comment:45 Changed 12 months ago by
- Commit changed from 96521a4ca9661dae2623c038517143c96f70e6c2 to e4f15922c8b7f49d5337c74b15dfb36bcdcc39bf
Branch pushed to git repo; I updated commit sha1. New commits:
e4f1592 | Merge branch 'develop' into t/24212/24212-2
|
comment:46 Changed 9 months ago by
- Branch changed from u/rws/24212-2 to t/rws/24212-2
- Commit e4f15922c8b7f49d5337c74b15dfb36bcdcc39bf deleted
The dependency has been approved and merged. Thanks.
I've merged with the latest development version and now i'm checking if the branch applies.
comment:47 Changed 9 months ago by
- Branch changed from t/rws/24212-2 to u/mforets/24212-2
- Commit set to 61b2f763f7787c2b803c5ea68c277fab78187963
comment:48 Changed 9 months ago by
- Status changed from needs_review to positive_review
$ ./sage -t src/sage/calculus/calculus.py src/sage/functions/* ... Git branch: t/24212/24212-2 Using --optional=mpir,python2,sage ... ---------------------------------------------------------------------- All tests passed! ---------------------------------------------------------------------- Total time for all tests: 106.7 seconds cpu time: 111.3 seconds cumulative wall time: 104.6 seconds
comment:49 Changed 9 months ago by
Quick question: do you want to change the test in calculus.py
? In particular, do you still want to test that Sage handles the case when something in sympy gives something Sage doesn't know how to handle?
comment:50 follow-up: ↓ 51 Changed 9 months ago by
Alright. I found this one:
sage: laplace(cos(-1/t), t, s, algorithm='sympy') ... AttributeError: Unable to convert SymPy result (=meijerg(((), ()), ((-1/2, 0, 1/2), (0,)), s**2/16)/4) into Sage
(side note: i didn't read the syntax of sympy's meijerg function, but here is a quick comparison to WA's answer)
comment:51 in reply to: ↑ 50 Changed 9 months ago by
comment:52 Changed 9 months ago by
- Status changed from positive_review to needs_review
comment:53 Changed 9 months ago by
- Commit changed from 61b2f763f7787c2b803c5ea68c277fab78187963 to f49ce18fd615bb4aad046006d57c5054e43b353e
Branch pushed to git repo; I updated commit sha1. New commits:
f49ce18 | add doctest with attributeerror
|
comment:54 Changed 9 months ago by
- Status changed from needs_review to positive_review
When we have #17970 we'll likely convert the G expression to the Fresnel expression in Sage.
comment:55 Changed 8 months ago by
- Branch changed from u/mforets/24212-2 to f49ce18fd615bb4aad046006d57c5054e43b353e
- Resolution set to fixed
- Status changed from positive_review to closed
This is a tentative implementation of sine Fresnel integral. The numerical evaluation relies on mpmath. I doubted about how to write the custom evaluation
_eval_
method. In fact I have two failing doctests. I expect:whereas i get
fresnel_sin(0)
in both cases. Any hints?