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 rws)

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 rws

  • Description modified (diff)

comment:2 Changed 15 months ago by mforets

  • Branch set to u/mforets/fresnel_integrals
  • Cc mforets added
  • Commit set to e52fb2eec054b050f2d1c578ef3e7bd647fa407e

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:

sage: fresnel_sin(0)
0
sage: fresnel_sin(x).subs(x==0)
0

whereas i get fresnel_sin(0) in both cases. Any hints?

comment:3 Changed 15 months ago by mforets

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 rws

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 rws

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 git

  • Commit changed from e52fb2eec054b050f2d1c578ef3e7bd647fa407e to f158660589867242a065c8995b16087755793d66

Branch pushed to git repo; I updated commit sha1. New commits:

f158660symbolics in fresnel_sin eval

comment:7 Changed 15 months ago by mforets

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 rws

You mean unsigned_infinity?

comment:9 follow-up: Changed 15 months ago by mforets

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 rws

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 git

  • Commit changed from f158660589867242a065c8995b16087755793d66 to ada47e4c62449f2c32c86ae0cb8fe8c9b5b23e7c

Branch pushed to git repo; I updated commit sha1. New commits:

ada47e4handle complex infinities

comment:12 Changed 15 months ago by git

  • Commit changed from ada47e4c62449f2c32c86ae0cb8fe8c9b5b23e7c to 869020b95344d81e5e4ff27f6f5829614ac588b5

Branch pushed to git repo; I updated commit sha1. New commits:

869020badd cosine Fresnel integral

comment:13 Changed 15 months ago by mforets

  • Authors set to Marcelo Forets
  • Status changed from new to needs_review

comment:14 Changed 15 months ago by mforets

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 git

  • Commit changed from 869020b95344d81e5e4ff27f6f5829614ac588b5 to e930137c2c2de67ad5f37b347ca79fd743ad484d

Branch pushed to git repo; I updated commit sha1. New commits:

e930137add sympy -> sage conversion, and doctest

comment:16 Changed 15 months ago by rws

See how easy it is to add conversions now.

comment:17 Changed 15 months ago by mforets

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 rws

  • Status changed from needs_review to needs_work

Patchbot has relevant doctest errors.

comment:19 Changed 15 months ago by git

  • Commit changed from e930137c2c2de67ad5f37b347ca79fd743ad484d to 2a0e907c8ca56d8a0a267b5ef39e8da30dc6b999

Branch pushed to git repo; I updated commit sha1. New commits:

2a0e907fix laplace test

comment:20 Changed 15 months ago by mforets

  • 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 rws

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 rws

  • 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 git

  • Commit changed from 2a0e907c8ca56d8a0a267b5ef39e8da30dc6b999 to e6b884e36444490289062a47b1d8fb52b65cf63c

Branch pushed to git repo; I updated commit sha1. New commits:

e6b884efix operatorname and add derivatives

comment:24 Changed 15 months ago by mforets

  • Status changed from needs_work to needs_review

comment:25 Changed 15 months ago by rws

  • Reviewers set to Ralf Stephan
  • Status changed from needs_review to positive_review

Thanks for your work.

comment:26 Changed 15 months ago by mforets

well, thanks for the review and all your help.

comment:27 Changed 15 months ago by vbraun

  • Status changed from positive_review to needs_work

Merge conflict

comment:28 Changed 14 months ago by rws

  • Branch changed from u/mforets/fresnel_integrals to u/rws/fresnel_integrals

comment:29 Changed 14 months ago by rws

  • 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:

669ea2224171: Formal set membership function
34c977324171: make RealSet inherit from Set_generic
af8271724171: remove RealSet import; SR set coercion doctests
592ce8724171: Py3 fixes
aa11a3eMerge branch 'develop' into t/24171/formal_set_membership_function
44c2bdbMerge 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 mforets

well done, thanks.

comment:31 Changed 14 months ago by vbraun

  • Status changed from positive_review to needs_work

See patchbot

comment:32 Changed 14 months ago by rws

The depended-on #24171 needs a patch so I'll squash it all, keeping authorship info correct.

comment:33 Changed 14 months ago by rws

Actually the fails are due to the blockers #24284 and #24378 but they are not reviewed yet.

comment:34 Changed 14 months ago by vbraun

  • Dependencies set to #24284 #24378

comment:35 Changed 14 months ago by rws

  • Branch changed from u/rws/fresnel_integrals to u/rws/24212

comment:36 Changed 14 months ago by rws

  • Branch changed from u/rws/24212 to u/rws/24212-1

comment:37 Changed 14 months ago by rws

  • Branch changed from u/rws/24212-1 to u/rws/24212-2

comment:38 Changed 14 months ago by rws

  • 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:

e3cd31a24212: Fresnel integrals

comment:39 Changed 14 months ago by vbraun

  • 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 rws

random_expr is really interesting. It should be made easier to debug, and errors caught/not caught should be reviewed because TypeErrors from ECL represent real bugs and RuntimeErrors 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 rws

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 rws

  • Dependencies set to #24425

comment:43 Changed 14 months ago by git

  • Commit changed from e3cd31afa599b019dfe2c3312844f20644d981d8 to 96521a4ca9661dae2623c038517143c96f70e6c2

Branch pushed to git repo; I updated commit sha1. New commits:

a17755c24425: Fix inherently failing random_expr doctest
96521a4Merge branch 't/24425/fix_inherently_failing_random_expr_doctest' into t/24212/24212-2

comment:44 Changed 14 months ago by rws

  • Status changed from needs_work to needs_review

Dependency needs review.

comment:45 Changed 12 months ago by git

  • Commit changed from 96521a4ca9661dae2623c038517143c96f70e6c2 to e4f15922c8b7f49d5337c74b15dfb36bcdcc39bf

Branch pushed to git repo; I updated commit sha1. New commits:

e4f1592Merge branch 'develop' into t/24212/24212-2

comment:46 Changed 9 months ago by mforets

  • 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 mforets

  • Branch changed from t/rws/24212-2 to u/mforets/24212-2
  • Commit set to 61b2f763f7787c2b803c5ea68c277fab78187963

New commits:

e3cd31a24212: Fresnel integrals
96521a4Merge branch 't/24425/fix_inherently_failing_random_expr_doctest' into t/24212/24212-2
e4f1592Merge branch 'develop' into t/24212/24212-2
61b2f76Merge branch 'develop' into t/24212/24212-2

comment:48 Changed 9 months ago by mforets

  • 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 tscrim

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: Changed 9 months ago by mforets

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 rws

Replying to mforets:

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

For that, there is #17970.

comment:52 Changed 9 months ago by mforets

  • Status changed from positive_review to needs_review

comment:53 Changed 9 months ago by git

  • Commit changed from 61b2f763f7787c2b803c5ea68c277fab78187963 to f49ce18fd615bb4aad046006d57c5054e43b353e

Branch pushed to git repo; I updated commit sha1. New commits:

f49ce18add doctest with attributeerror

comment:54 Changed 9 months ago by rws

  • 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 vbraun

  • Branch changed from u/mforets/24212-2 to f49ce18fd615bb4aad046006d57c5054e43b353e
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.