Opened 7 years ago
Closed 3 years ago
#4498 closed enhancement (fixed)
Implement a symbolic version of the arg function
Reported by: | TimothyClemans | Owned by: | somebody |
---|---|---|---|
Priority: | major | Milestone: | sage-5.0 |
Component: | symbolics | Keywords: | beginner, sd35.5 |
Cc: | kcrisman, ktkohl | Merged in: | sage-5.0.beta4 |
Authors: | Karen T. Kohl, Burcin Erocal | Reviewers: | Karl-Dieter Crisman, Burcin Erocal |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by burcin)
It would be nice if there were a symbolic arg function, just like the symbolic sin, cos, etc., functions. Then the following would happen:
sage: f = arg(x); f arg(x) sage: f.subs(x=1+I) arg(1+I)
Now we have
sage: arg(1+I) 0.785398163397 sage: type(arg(1+I)) <type 'sage.rings.real_double.RealDoubleElement'>
I.e., the arg in Sage is currently the numerical person's arg, not the symbolic person's. It just casts to CDF.
"The function should return the argument of a complex function." - Ronan Paixão
Apply
Attachments (5)
Change History (30)
comment:1 Changed 7 years ago by TimothyClemans
comment:2 Changed 7 years ago by mabshoff
Please post a complete session. As is the above is not very clear.
Cheers,
Michael
comment:3 Changed 7 years ago by was
- Description modified (diff)
- Milestone changed from sage-3.2.1 to sage-wishlist
- Summary changed from The argument function does not work with variables. to Implement a symbolic version of the arg function; the current one is only for actual numbers
- Type changed from defect to enhancement
comment:4 Changed 7 years ago by ronanpaixao
Is this really an enchancement? As it is, just using arg(x) already raises an error and everything that needs arg() must be implemented numerically.
comment:5 Changed 7 years ago by was
Is this really an enchancement? As it is, just using arg(x) already
raises an error and everything that needs arg() must be implemented numerically.
Yes, this is an enhancement since it is implementing new functionality.
It would be a bug fix if there were a bug in the existing arg function, where it produced invalid results on supported input.
-- William
comment:6 Changed 5 years ago by kcrisman
- Cc kcrisman added
- Component changed from basic arithmetic to symbolics
- Report Upstream set to N/A
comment:7 Changed 5 years ago by burcin
- Summary changed from Implement a symbolic version of the arg function; the current one is only for actual numbers to Implement a symbolic version of the arg function
This was also reported in #6220. I will close that as duplicate.
comment:8 Changed 5 years ago by burcin
- Keywords beginner added
comment:9 Changed 3 years ago by ktkohl
- Cc ktkohl added
- Keywords sd35.5 added
comment:10 Changed 3 years ago by ktkohl
- Status changed from new to needs_review
comment:11 Changed 3 years ago by kcrisman
- Reviewers set to Karl-Dieter Crisman
- Status changed from needs_review to needs_work
There are a few small formatting issues, and it would be good to add an example showing that arg(sqrt(2)+i) remaining symbolic (as opposed to arctan(1/sqrt(2))) still evaluates correctly numerically. Otherwise looks fine. Currently running tests, as arg is likely used in a lot of places in Sage...
comment:12 Changed 3 years ago by kcrisman
File "/Users/karl-dietercrisman/Downloads/sage-4.8.alpha5/devel/sage-main/sage/symbolic/random_tests.py", line 16: sage: [f for (one,f,arity) in _mk_full_functions()] Expected: [Ei, abs, arccos, arccosh, arccot, arccoth, arccsc, arccsch, arcsec, arcsech, arcsin, arcsinh, arctan, arctan2, arctanh, binomial, ceil, conjugate, cos, cosh, cot, coth, csc, csch, dickman_rho, dilog, dirac_delta, elliptic_e, elliptic_ec, elliptic_eu, elliptic_f, elliptic_kc, elliptic_pi, erf, exp, factorial, floor, heaviside, imag_part, integrate, kronecker_delta, log, polylog, real_part, sec, sech, sgn, sin, sinh, tan, tanh, unit_step, zeta, zetaderiv] Got: [Ei, abs, arccos, arccosh, arccot, arccoth, arccsc, arccsch, arcsec, arcsech, arcsin, arcsinh, arctan, arctan2, arctanh, arg, binomial, ceil, conjugate, cos, cosh, cot, coth, csc, csch, dickman_rho, dilog, dirac_delta, elliptic_e, elliptic_ec, elliptic_eu, elliptic_f, elliptic_kc, elliptic_pi, erf, exp, factorial, floor, heaviside, imag_part, integrate, kronecker_delta, log, polylog, real_part, sec, sech, sgn, sin, sinh, tan, tanh, unit_step, zeta, zetaderiv] ********************************************************************** File "/Users/karl-dietercrisman/Downloads/sage-4.8.alpha5/devel/sage-main/sage/symbolic/random_tests.py", line 238: sage: random_expr(5, verbose=True) Expected: About to apply dirac_delta to [1] About to apply arccsch to [0] About to apply <built-in function add> to [0, arccsch(0)] arccsch(0) Got: About to apply dirac_delta to [1] About to apply arcsec to [0] About to apply <built-in function add> to [0, arcsec(0)] arcsec(0)
comment:13 Changed 3 years ago by kcrisman
I think the Maxima translation may not be correct.
-- Function: carg (<z>) Returns the complex argument of <z>. The complex argument is an angle `theta' in `(-%pi, %pi]' such that `r exp (theta %i) = <z>' where `r' is the magnitude of <z>. `carg' is a computational function, not a simplifying function. See also `abs' (complex magnitude), `polarform', `rectform', `realpart', and `imagpart'. Examples: (%i1) carg (1); (%o1) 0 (%i2) carg (1 + %i); %pi (%o2) --- 4 (%i3) carg (exp (%i)); (%o3) 1 (%i4) carg (exp (%pi * %i)); (%o4) %pi (%i5) carg (exp (3/2 * %pi * %i)); %pi (%o5) - --- 2 (%i6) carg (17 * exp (2 * %i)); (%o6) 2 (%o3) true
See also Barton Willis' parg, though that only works if having loaded to_poly_solve.
comment:14 Changed 3 years ago by kcrisman
- Work issues set to random tests, Maxima
Otherwise, all tests pass!
comment:15 Changed 3 years ago by kcrisman
Do this to check the fix works.
sage: maxima(arg(x)) atan2(0,x) sage: maxima(arg(2+i)) atan(1/2) sage: maxima(arg(sqrt(2)+i)) atan(1/sqrt(2)) sage: arg(2+i) arctan(1/2) sage: arg(sqrt(2)+i) arg(sqrt(2) + I)
It also seems to help with the sqrt(2) issue, in a manner of speaking. One could tell someone to do
sage: arg(sqrt(2)+i).simplify() arctan(1/2*sqrt(2))
comment:16 Changed 3 years ago by ktkohl
- Status changed from needs_work to needs_review
Changed 3 years ago by burcin
comment:17 Changed 3 years ago by burcin
- Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Burcin Erocal
I uploaded a new patch with minor modifications to Karen's. In particular, it
- removes the duplicate commands that appear in the EXAMPLES and TESTS blocks for Function_arg.
- import CC directly instead of calling ComplexField in _evalf_().
I give a positive review to Karen's patch. If someone can take a quick look at my changes to check if I didn't mess anything up, this can be merged.
comment:18 Changed 3 years ago by kcrisman
- Status changed from needs_review to needs_work
It doesn't appear that you messed anything up, except ...
sage: arg(3.0) --------------------------------------------------------------------------- <snip> 1426 return x.arg() 1427 except AttributeError: -> 1428 from sage.rings.complex_field import CC 1429 x = CC(x) 1430 return x.arg() ImportError: cannot import name CC
So apparently that won't work. Otherwise the changes are fine.
Changed 3 years ago by burcin
comment:19 Changed 3 years ago by burcin
- Description modified (diff)
- Status changed from needs_work to needs_review
Apparently I didn't run tests, latex output was also broken.
attachment:trac_4498-arg_evalf.patch, to be applied after attachment:trac_4498-symbolic_arg.cleanup.patch, implements a new _evalf_() function which keeps the precision of the input. This one needs a real review. :)
comment:20 Changed 3 years ago by kcrisman
This makes a lot more sense. Running tests...
Why parent_d and not parent? Just wondering in case there is a convention I should be aware of.
comment:21 Changed 3 years ago by burcin
- Milestone changed from sage-wishlist to sage-5.0
Using parent as the name of the keyword argument masks the imported parent() function, which I used in the function body.
comment:22 Changed 3 years ago by kcrisman
- Status changed from needs_review to positive_review
I wondered; that makes sense.
All looks well. Just a question - do you want to include any of the following as tests?
sage: arg(long(1000)) 0 sage: arg(1j) 1.57079632679490 sage: arg(1J) 1.57079632679490 sage: arg(complex(0,1)) 1.57079632679490 sage: arg(complex(1,0)) 0.000000000000000 sage: arg(int(10)) 0
It's not a big deal to me either way, I just wanted to test them.
comment:23 Changed 3 years ago by jdemeyer
- Status changed from positive_review to needs_work
comment:24 Changed 3 years ago by jdemeyer
- Status changed from needs_work to positive_review
- Work issues random tests, Maxima deleted
comment:25 Changed 3 years ago by jdemeyer
- Merged in set to sage-5.0.beta4
- Resolution set to fixed
- Status changed from positive_review to closed