Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#6803 closed enhancement (fixed)

Implement symbolic Kronecker delta and also make current signum (sgn) symbolic

Reported by: gmhossain Owned by:
Priority: major Milestone: sage-4.3
Component: symbolics Keywords:
Cc: Merged in: sage-4.3.alpha0
Authors: Golam Mortuza Hossain Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by gmhossain)

We should have a symbolic Kronecker delta in Sage.

Also, current implementation of signum (sgn) function returns wrong answer for symbolic input.

sage: sgn(x)
1
sage: sgn(-x)
1

So we should make sgn() symbolic aware. Given, implementation of these functions in new symbolics should be very similar to the existing generalized function Dirac delta and Heaviside, I am putting them together here.

Also, ticket #777 should be closed down as Sign is already in Sage and this ticket will further enhance it.

Patch:

Apart from implementing these two symbolic functions, attached patch slightly speeds up three other generalized functions by avoiding default __call__ method of PrimitiveFunction?. These functions take explicit value either 0,-1,1 so those checks are not needed.

Timing before the patches:

sage: timeit('dirac_delta(1.0)')
625 loops, best of 3: 179 µs per loop
sage: timeit('unit_step(1.0)')
625 loops, best of 3: 345 µs per loop
sage: timeit('heaviside(1.0)')
625 loops, best of 3: 344 µs per loop

Timing after the patches:

sage: timeit('dirac_delta(1.0)')
625 loops, best of 3: 159 µs per loop
sage: timeit('heaviside(1.0)')
625 loops, best of 3: 324 µs per loop
sage: timeit('unit_step(1.0)')
625 loops, best of 3: 323 µs per loop

Also, it does slight re-arrangements of references.

Attachments (3)

trac_6803-symbolic-kronecker-n-signum.patch (9.7 KB) - added by gmhossain 12 years ago.
trac_6803-symbolic-kronecker-n-signum-review.patch (10.9 KB) - added by kcrisman 12 years ago.
Based on 4.2.1.alpha0, apply only this patch.
trac_6803-sgn.patch (618 bytes) - added by mvngu 11 years ago.
apply after previous

Download all attachments as: .zip

Change History (10)

Changed 12 years ago by gmhossain

comment:1 Changed 12 years ago by gmhossain

  • Authors set to Golam Mortuza Hossain
  • Description modified (diff)
  • Summary changed from Implement symbolic Kronecker delta and also make current signum (sgn) symbolic to [with patch, needs review] Implement symbolic Kronecker delta and also make current signum (sgn) symbolic

comment:2 Changed 12 years ago by kcrisman

  • Milestone set to sage-4.1.2
  • Reviewers set to Karl-Dieter Crisman
  • Summary changed from [with patch, needs review] Implement symbolic Kronecker delta and also make current signum (sgn) symbolic to [with patch, needs work] Implement symbolic Kronecker delta and also make current signum (sgn) symbolic

Overall this is good, and should get positive review. However, there is a doctest failure (very typical when one adds a new symbolic function in line 204 of symbolic/random_tests.py, with random_expr(). This should be easy to fix.

comment:3 Changed 12 years ago by kcrisman

  • Status changed from needs_work to needs_review
  • Summary changed from [with patch, needs work] Implement symbolic Kronecker delta and also make current signum (sgn) symbolic to Implement symbolic Kronecker delta and also make current signum (sgn) symbolic

I have attached the patch, but with the random test fixed as a reviewer patch. Apply only this patch.

Changed 12 years ago by kcrisman

Based on 4.2.1.alpha0, apply only this patch.

comment:4 Changed 12 years ago by kcrisman

  • Status changed from needs_review to positive_review

comment:5 follow-up: Changed 11 years ago by mhansen

  • Merged in set to sage-4.3.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

I had to add a small patch to the doctest for sage.quadratic_forms.extras.sgn to make sure that the doctest was actually doctesting that function.

comment:6 in reply to: ↑ 5 ; follow-up: Changed 11 years ago by kcrisman

Replying to mhansen:

I had to add a small patch to the doctest for sage.quadratic_forms.extras.sgn to make sure that the doctest was actually doctesting that function.

Can you post that here, or at least the code in braces - just for posterity?

Changed 11 years ago by mvngu

apply after previous

comment:7 in reply to: ↑ 6 Changed 11 years ago by mvngu

  • Report Upstream set to N/A

Replying to kcrisman:

Can you post that here, or at least the code in braces - just for posterity?

mhansen's patch is contained in trac_6803-sgn.patch

Note: See TracTickets for help on using tickets.