Removed useless argument, added tests for wigner.racah(), Fixed error for half integers

[debugger22]

Added tests for racah() of sympy.physics.wigner module which was not added in #2878
Removed useless prec argument from clebsch_gordan and wigner_3j.
Updated docstrings.
Fixes #2767
Fixes #2766

[akshayah3]

#2985 Check this out, Prec has already been removed here.

[debugger22]

@akshayah3 Oh! I didn't know that. Anyway that PR hasn't been merged yet and I've also done few more things beside that.

[akshayah3]

Anyway I have referenced it, you two can sort it out.

[debugger22]

@flacjacket I've no idea why the Travis build failed. I checked it my computer and it is working fine.

[jcrist]

@debugger22 if you look into the travis build, it gives 2 reasons:
1.) You forgot to remove a "prec" in clebsch_gordan on line 261.
2.) You have trailing whitespace.

[debugger22]

Thanks! I'll fix it ASAP.
On Mar 4, 2014 10:06 PM, "Jim Crist" notifications@github.com wrote:

@debugger22 https://github.com/debugger22 if you look into the travis
build, it gives 2 reasons:
1.) You forgot to remove a "prec" in clebsch_gordan on line 261.
2.) You have trailing whitespace.

Reply to this email directly or view it on
GitHubhttps://github.com//pull/2992#issuecomment-36644451
.

[debugger22]

@jcrist I fixed those things what u had pointed. The build is not triggering now, is that an issue or just a propagation delay?

[akshayah3]

Its a propagation delay.

[flacjacket]

Hmm, I hadn't noticed that racah didn't directly have any tests at all, except indirectly through wigner_6j.

You should also add some tests that fail before 0cae01c, but have been fixed by the int(...) changes to really say 2767 is fixed, so you need some tests that have float arguments. You should find, in fact, there are still issues. For example, you have added a test for racah(3, 3, 3, 3, 3, 3), but changing to floats, racah(3., 3., 3., 3., 3., 3.) throws an error (same is true for the existing 6j/9j/gaunt tests). It would be good to also check the result for half-integer values, if you have trouble finding ones that work, I can try to look some up, I have a book with tables of them back at my house.

[certik]

@debugger22 I just merged #2985. Would you mind merging with master? It shouldn't introduce any major conflicts. Your PR adds some good additional stuff that we should merge.

[debugger22]

@certik Sure! no problem, I'll rebase it first.

[debugger22]

@certik @flacjacket Please review this. Done rebasing with master and added test for half integers for which racah() was raising error earlier

[certik]

Funny, github is somehow confused by this PR. But checking out your branch by hand and doing:

ondrej@kittiwake:~/repos/sympy((f92fb93...))$ git diff master

produces:

diff --git a/sympy/physics/tests/test_clebsch_gordan.py b/sympy/physics/tests/test_clebsch_gordan.py
index 6fc1027..60ffa2e 100644
--- a/sympy/physics/tests/test_clebsch_gordan.py
+++ b/sympy/physics/tests/test_clebsch_gordan.py
@@ -1,5 +1,5 @@
 from sympy import S, sqrt, pi
-from sympy.physics.wigner import clebsch_gordan, wigner_9j, wigner_6j, gaunt
+from sympy.physics.wigner import clebsch_gordan, wigner_9j, wigner_6j, gaunt, racah
 from sympy.core.numbers import Rational

 # Todo: more tests should be added from:
@@ -120,3 +120,9 @@ def tn(a, b):
     assert gaunt(1, 0, 1, 1, 0, -1) == -1/(2*sqrt(pi))
     assert tn(gaunt(
         10, 10, 12, 9, 3, -12, prec=64), (-S(98)/62031) * sqrt(6279)/sqrt(pi))
+
+def test_racah():
+    assert racah(3,3,3,3,3,3) == Rational(-1,14)
+    assert racah(2,2,2,2,2,2) == Rational(-3,70)
+    assert racah(7,8,7,1,7,7, prec=4).is_Float
+    assert racah(5.5,7.5,9.5,6.5,8,9)
diff --git a/sympy/physics/wigner.py b/sympy/physics/wigner.py
index bc66615..827b51f 100644
--- a/sympy/physics/wigner.py
+++ b/sympy/physics/wigner.py
@@ -65,7 +65,7 @@ def _calc_factlist(nn):
         [1, 1, 2, 6, 24, 120, 720, 5040, 40320, 362880, 3628800]
     """
     if nn >= len(_Factlist):
-        for ii in range(len(_Factlist), nn + 1):
+        for ii in range(len(_Factlist), int(nn + 1)):
             _Factlist.append(_Factlist[ii - 1] * ii)
     return _Factlist[:int(nn) + 1]

@@ -226,7 +226,7 @@ def clebsch_gordan(j_1, j_2, j_3, m_1, m_2, m_3):

     OUTPUT:

-    Rational number times the square root of a rational number
+    Rational number times the square root of a rational number.

     EXAMPLES::

@@ -288,6 +288,7 @@ def _big_delta_coeff(aa, bb, cc, prec=None):
         sage: _big_delta_coeff(1,1,1)
         1/2*sqrt(1/6)
     """
+
     if int(aa + bb - cc) != (aa + bb - cc):
         raise ValueError("j values must be integer or half integer and fulfill the triangle relation")
     if int(aa + cc - bb) != (aa + cc - bb):
@@ -532,7 +533,7 @@ def wigner_9j(j_1, j_2, j_3, j_4, j_5, j_6, j_7, j_8, j_9, prec=None):
     imax = min(j_1 + j_9, j_2 + j_6, j_4 + j_8)

     sumres = 0
-    for kk in range(int(imin), int(imax) + 1):
+    for kk in range(imin, int(imax) + 1):
         sumres = sumres + (2 * kk + 1) * \
             racah(j_1, j_2, j_9, j_6, j_3, kk, prec) * \
             racah(j_4, j_6, j_8, j_2, j_5, kk, prec) * \

Which is 👍 from me. Let's wait for tests to finish and if all is ok, merge.

[certik]

In here, in addition shouldn't we test the actual result?

In [1]: from sympy.physics.wigner import racah

In [2]: racah(5.5,7.5,9.5,6.5,8,9)
Out[2]: 
       _____ 
-719⋅╲╱ 598  
─────────────
   1158924   

In [3]: racah(5.5,7.5,9.5,6.5,8,9, prec=4)
Out[3]: -0.01517

i.e. test within some eps etc.

[debugger22]

I am new to sympy, I don't know how to represent those numbers. I tried doing what you just said like above two cases.

[certik]

Ah I see. The number

       _____ 
-719⋅╲╱ 598  
─────────────
   1158924   

is represented like this:

In [4]: -719*sqrt(598)/1158924
Out[4]: 
       _____ 
-719⋅╲╱ 598  
─────────────
   1158924   

and as to the float, just do something like:

assert abs(racah(5.5,7.5,9.5,6.5,8,9, prec=4) - (-0.01517)) < 1e-4

[debugger22]

Thanks! I'm on it.

[debugger22]

Done! 😄

[certik]

Yep, it works now. Btw, the numerical value is obtained as:

In [18]: (-719*sqrt(598)/1158924).n()
Out[18]: -0.0151713604143125

But that doesn't matter. 👍 to merge if tests pass.

[debugger22]

Build passed at last :)

[certik]

Cool, merging.