Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wip: add coverage testing #29

Merged
merged 1 commit into from
Dec 14, 2022

Conversation

oscarbenjamin
Copy link
Collaborator

Work in progress to add coverage testing.

Currently requires a patch to cython due to the use of include although that wouldn't be needed if #15 was fixed.

Current coverage report:

$ coverage report -m
Name                      Stmts   Miss  Cover   Missing
-------------------------------------------------------
src/flint/__init__.py         1      0   100%
src/flint/acb.pyx           891    874     2%   1, 5-6, 12-40, 44-49, 53-97, 126, 129, 131-189, 196-2562
src/flint/arb.pyx           835    773     7%   1-8, 11, 17-37, 45-46, 49-50, 53-54, 57-58, 61-62, 77-78, 92, 96-101, 105-110, 117-133, 185, 187-462, 471, 474, 487-496, 503-2423
src/flint/arf.pyx           109     98    10%   28, 30, 32, 37, 39-171
src/flint/fmpq.pyx          181     89    51%   1, 38-44, 48, 53, 56, 67, 70, 78-83, 101-102, 106, 121, 148, 151, 160, 163, 172, 180-181, 185, 188, 199-331
src/flint/fmpq_mat.pyx      248    105    58%   1, 6, 62, 64-65, 71, 78-81, 86, 89, 92, 98, 101, 109, 116-120, 150, 153, 158, 170, 173, 178, 184, 191, 199, 202, 208, 211, 217, 220, 240, 251-254, 257, 264, 297-455
src/flint/fmpq_poly.pyx     234    108    54%   1, 21, 35, 71, 75, 77, 83, 86, 92, 95, 98, 114-115, 121, 127, 136, 159-172, 183, 186, 195, 198, 207, 210, 217-244, 250, 256, 263-274, 279, 294-379
src/flint/fmpz.pyx          340    177    48%   1, 11, 13-14, 18-20, 26-29, 33, 38-40, 47, 85, 88-92, 99-102, 112-118, 131, 135, 154, 292, 309-633
src/flint/fmpz_mat.pyx      268    135    50%   1, 92, 101, 108, 116, 119, 122, 128, 134, 145, 152-156, 195, 198, 213-229, 245, 262-263, 270, 280, 286-342, 358-359, 374-375, 434, 440, 442-446, 466-706
src/flint/fmpz_poly.pyx     295    162    45%   1, 10-12, 20, 32-33, 62, 67, 70, 76, 79, 135-140, 154, 157, 166, 169, 178, 190, 193, 204, 207, 218, 221, 224-227, 232, 247-505
src/flint/nmod.pyx           96     17    82%   1, 21, 42-45, 54, 63, 74-77, 95, 112, 124-127, 135, 140, 146, 156
src/flint/nmod_mat.pyx      222    100    55%   1, 8-12, 38, 42-44, 62, 64-65, 71, 77, 86, 89, 96-98, 101, 104, 107-115, 119, 149-164, 197, 200-204, 206, 218-239, 256, 264-265, 267, 269, 275-276, 279, 287, 292, 297, 310-411
src/flint/nmod_poly.pyx     199     82    59%   1, 18-20, 32, 65-69, 72, 79-84, 87, 90, 97, 107, 119-127, 133, 139-147, 161-164, 177, 181, 183, 194, 198, 200, 211, 215, 217, 230-234, 236, 238, 249-253, 255, 257, 271, 287-342
src/flint/pyflint.pyx       132     27    80%   35, 41, 95-103, 109, 113, 122-131, 142, 148-150, 190, 197, 200, 210, 220, 235, 266-267, 270, 288-293
test/test.py                436      2    99%   16, 454
-------------------------------------------------------
TOTAL                      4487   2749    39%

Note that that does not include files that have literally zero coverage:

$ ls src/flint/*.pyx
src/flint/acb_mat.pyx     src/flint/arb_mat.pyx     src/flint/arf.pyx        src/flint/fmpq.pyx         src/flint/fmpz_poly.pyx    src/flint/nmod_mat.pyx     src/flint/pyflint.pyx
src/flint/acb_poly.pyx    src/flint/arb_poly.pyx    src/flint/dirichlet.pyx  src/flint/fmpq_series.pyx  src/flint/fmpz.pyx         src/flint/nmod_poly.pyx
src/flint/acb.pyx         src/flint/arb.pyx         src/flint/fmpq_mat.pyx   src/flint/fmpz_mat.pyx     src/flint/fmpz_series.pyx  src/flint/nmod.pyx
src/flint/acb_series.pyx  src/flint/arb_series.pyx  src/flint/fmpq_poly.pyx  src/flint/fmpz_mpoly.pyx   src/flint/functions.pyx    src/flint/nmod_series.pyx
$ ls src/flint/*.pyx | wc -l
25
$ coverage report -m | grep pyx | wc -l
13

@fredrik-johansson
Copy link
Collaborator

Note that that does not include files that have literally zero coverage:

There should be at least some coverage if one includes doctests?

@oscarbenjamin
Copy link
Collaborator Author

There should be at least some coverage if one includes doctests?

I'm running test/test.py which at least looks like it's intended to run the doctests:
https://github.com/fredrik-johansson/python-flint/blob/edf6be7bd7c2b8676871357957a2bcfb1c000759/test/test.py#L450

@oscarbenjamin
Copy link
Collaborator Author

Maybe this is the doctest problem:

In [11]: import flint

In [12]: import doctest

In [13]: doctest.testmod(flint)
Out[13]: TestResults(failed=0, attempted=0)

@oscarbenjamin
Copy link
Collaborator Author

Okay, here we go:

In [23]: doctest.testmod(flint)
Out[23]: TestResults(failed=0, attempted=0)

In [24]: doctest.testmod(flint._flint)
**********************************************************************
File "/home/oscar/current/sympy/python-flint/src/flint/_flint.cpython-38-x86_64-linux-gnu.so", line ?, in flint._flint.__test__.acb_mat.eig (line 620)
Failed example:
    showgood(lambda: sum(acb_mat(arb_mat.hilbert(20,20)).eig(nonstop=True)), parts=False)
Expected:
    2.47967321036454 + [+/- 1.48e-56]j
Got:
    2.47967321036454 + 0e-55j
**********************************************************************
File "/home/oscar/current/sympy/python-flint/src/flint/_flint.cpython-38-x86_64-linux-gnu.so", line ?, in flint._flint.__test__.arb_mat.chop (line 613)
Failed example:
    print(arb_mat.stirling(4, 4).inv().str(5, radius=False))
Expected:
    [1.0000,       0,              0,              0]
    [     0,  1.0000, [+/- 1.20e-15], [+/- 5.00e-16]]
    [     0, -1.0000,         1.0000, [+/- 1.67e-16]]
    [     0,  1.0000,        -3.0000,         1.0000]
Got:
    [1.0000,       0,       0,      0]
    [     0,  1.0000,   0e-14,  0e-15]
    [     0, -1.0000,  1.0000,  0e-15]
    [     0,  1.0000, -3.0000, 1.0000]
**********************************************************************
File "/home/oscar/current/sympy/python-flint/src/flint/_flint.cpython-38-x86_64-linux-gnu.so", line ?, in flint._flint.acb_mat.eig
Failed example:
    showgood(lambda: sum(acb_mat(arb_mat.hilbert(20,20)).eig(nonstop=True)), parts=False)
Expected:
    2.47967321036454 + [+/- 1.48e-56]j
Got:
    2.47967321036454 + 0e-55j
**********************************************************************
File "/home/oscar/current/sympy/python-flint/src/flint/_flint.cpython-38-x86_64-linux-gnu.so", line ?, in flint._flint.arb_mat.chop
Failed example:
    print(arb_mat.stirling(4, 4).inv().str(5, radius=False))
Expected:
    [1.0000,       0,              0,              0]
    [     0,  1.0000, [+/- 1.20e-15], [+/- 5.00e-16]]
    [     0, -1.0000,         1.0000, [+/- 1.67e-16]]
    [     0,  1.0000,        -3.0000,         1.0000]
Got:
    [1.0000,       0,       0,      0]
    [     0,  1.0000,   0e-14,  0e-15]
    [     0, -1.0000,  1.0000,  0e-15]
    [     0,  1.0000, -3.0000, 1.0000]
**********************************************************************
4 items had failures:
   1 of  21 in flint._flint.__test__.acb_mat.eig (line 620)
   1 of   2 in flint._flint.__test__.arb_mat.chop (line 613)
   1 of  21 in flint._flint.acb_mat.eig
   1 of   2 in flint._flint.arb_mat.chop
***Test Failed*** 4 failures.
Out[24]: TestResults(failed=4, attempted=1568)

Looks like the doctests were not being run before. They take a lot longer than the other tests. Perhaps that makes they're more comprehensive...

This diff is needed:

diff --git a/test/test.py b/test/test.py
index 88f781b..3f0d741 100644
--- a/test/test.py
+++ b/test/test.py
@@ -447,7 +447,7 @@ if __name__ == "__main__":
     sys.stdout.write("test_nmod_mat..."); test_nmod_mat(); print("OK")
     sys.stdout.write("test_arb.."); test_arb(); print("OK")
     sys.stdout.write("doctests...");
-    fail, total = doctest.testmod(flint);
+    fail, total = doctest.testmod(flint._flint);
     if fail == 0:
         print("OK")
     else:

With that we have the failures shown above and coverage is:

$ coverage report -m
Name                        Stmts   Miss  Cover   Missing
---------------------------------------------------------
src/flint/__init__.py           1      0   100%
src/flint/acb.pyx             891    176    80%   1, 21-29, 38-40, 46, 49, 53-76, 82-83, 89, 91-94, 97, 126, 129, 131-140, 156-181, 188-189, 196, 237-242, 260-266, 274-293, 371, 374, 387, 390, 403, 406, 414-415, 421, 424, 435, 442, 445, 448, 456-459, 668-677, 1079-1097, 1286, 1492, 1517, 1522-1523, 1553, 1572, 1759-1760, 1774, 1791-1792, 1822, 1875-1876, 1890-1891, 1904-1905, 1920-1921, 1935-1936, 1950-1951, 1966-1967, 1982-1983, 1996-1997, 2183, 2196-2198, 2346-2347, 2362, 2367-2368, 2392, 2403-2404, 2471, 2473, 2475, 2477, 2479, 2486-2488, 2493-2494, 2509
src/flint/acb_mat.pyx         370    114    69%   1, 4, 7-14, 17, 37-38, 45-50, 57, 78-81, 83-86, 92, 96, 103-112, 127, 133-138, 144, 155, 164, 173-176, 208, 215-225, 233, 240, 249, 254-257, 270, 280-286, 295-296, 301, 303, 309, 316, 320, 322, 353, 394, 399-406, 413, 426, 441, 468, 473-474, 533, 568-578, 721, 735-738, 752, 762, 770
src/flint/acb_poly.pyx        233    179    23%   1-26, 47-65, 68, 79-89, 119-289, 318, 342, 366-371
src/flint/arb.pyx             835    180    78%   1, 8, 17-37, 53-54, 61-62, 77-78, 92, 99-101, 107, 110, 117-119, 125, 128, 185, 192, 195, 198-201, 235-250, 354-374, 458, 462, 471, 474, 487-496, 503-529, 558, 561, 574, 577, 590, 593, 604, 611, 614, 617, 870-871, 882-883, 894-895, 964-965, 1130-1131, 1203-1204, 1281-1282, 1301-1302, 1318-1319, 1360-1361, 1389-1390, 1401-1402, 1454, 1473, 1493-1494, 1509, 1516, 1519-1520, 1535, 1542, 1639, 1660, 1665-1666, 1694, 2080-2081, 2092-2093, 2104-2105, 2116-2117, 2128-2129, 2140-2141, 2152-2153, 2164-2165, 2176-2177, 2188-2204, 2226, 2245, 2251-2254, 2326-2327
src/flint/arb_mat.pyx         323    127    61%   1, 4, 7-18, 21-23, 47-48, 55-60, 67, 81-84, 86-89, 95, 99, 106-115, 130, 136-141, 147, 158, 167, 176-179, 198, 205-215, 223, 230, 235-247, 260, 270-276, 285-286, 291, 293, 299, 306, 310, 312, 342, 387, 392-399, 419, 434, 461, 466-467, 487-488, 517-518, 548, 554-555, 611, 649-667
src/flint/arb_poly.pyx        195    170    13%   1-14, 26-27, 41-57, 60-61, 71-81, 114-290
src/flint/arb_series.pyx      563    492    13%   1-12, 30, 32, 35-36, 38-39, 41, 43, 51-54, 60, 65-345, 358-476, 489-740, 776, 781, 802
src/flint/arf.pyx             109     85    22%   28, 37, 40-47, 53-89, 96-103, 108, 114-171
src/flint/dirichlet.pyx        56     27    52%   1, 27-28, 35, 41, 102-142, 155-161
src/flint/fmpq.pyx            181     67    63%   1, 43-44, 48, 53, 56, 67, 70, 78-83, 101-102, 106, 148, 151, 160, 163, 172, 180-181, 185, 188, 199, 239, 246-247, 262-263, 268-269, 282-331
src/flint/fmpq_mat.pyx        248     70    72%   1, 6, 62, 71, 78-81, 86, 89, 92, 98, 101, 109, 116-120, 150, 153, 158, 170, 173, 178, 184, 191, 199, 202, 208, 211, 217, 220, 240, 251-254, 257, 264, 335, 346, 348, 352, 386-387, 423-455
src/flint/fmpq_poly.pyx       234     75    68%   1, 21, 35, 71, 75, 77, 83, 86, 92, 95, 98, 114-115, 121, 127, 136, 159-172, 183, 186, 195, 198, 207, 210, 217-244, 250, 256, 265, 268, 270, 279, 296, 303-310, 344-345, 356-357, 368-369
src/flint/fmpq_series.pyx     349    265    24%   1, 7-20, 38, 40, 43-44, 46-47, 49, 51, 58-63, 68, 72-83, 89, 94-101, 107-141, 155-158, 171-176, 179, 185-186, 195, 200-201, 207, 210, 215-222, 229, 236, 241, 244, 246, 249-269, 282, 284, 292-487
src/flint/fmpz.pyx            340    110    68%   1, 11, 13-14, 18-20, 26-29, 33, 38-40, 47, 85, 90, 92, 99-102, 112-118, 131, 135, 292, 312, 327-335, 390-411, 445-446, 457-458, 470-471, 493-503, 514-515, 526-527, 538-539, 579-633
src/flint/fmpz_mat.pyx        268     59    78%   1, 92, 101, 108, 116, 119, 122, 128, 134, 145, 154, 195, 198, 213-229, 262-263, 270, 280, 286-287, 341-342, 358-359, 374-375, 434, 440, 510, 530, 610-613, 616-619, 696-706
src/flint/fmpz_poly.pyx       295     97    67%   1, 10-12, 20, 32-33, 62, 67, 70, 76, 79, 135-140, 154, 157, 166, 169, 178, 190, 193, 204, 207, 218, 221, 232, 249, 316, 333-334, 346-347, 359-360, 373-374, 387-388, 405, 411-412, 415, 418-419, 441-505
src/flint/functions.pyx        61     12    80%   2, 4, 14-16, 20, 25-27, 69, 79, 84, 88, 102
src/flint/nmod.pyx             96     16    83%   1, 21, 42, 54, 63, 74-77, 95, 112, 124-127, 135, 140, 146, 156
src/flint/nmod_mat.pyx        222     74    67%   1, 8-12, 38, 42-44, 62, 64-65, 71, 77, 86, 89, 96-98, 101, 104, 107-115, 149-164, 197, 200-204, 206, 218-239, 256, 264-265, 267, 269, 275-276, 279, 287, 292, 297, 343
src/flint/nmod_poly.pyx       199     49    75%   1, 18-20, 32, 65-69, 72, 79, 84, 87, 90, 97, 107, 133, 139-147, 161-164, 177, 181, 183, 194, 198, 200, 211, 215, 217, 230-234, 236, 238, 249-253, 255, 257, 271, 289, 291
src/flint/pyflint.pyx         132     15    89%   35, 41, 109, 122-131, 142, 148, 220, 235, 266-267, 270, 293
test/test.py                  436      2    99%   16, 452
---------------------------------------------------------
TOTAL                        6637   2461    63%
$ coverage report -m | grep pyx | wc -l
21
$ ls src/flint/*.pyx | wc -l
25
$ ls src/flint
acb_mat.pyx     arb_series.pyx                         fmpq.pyx         functions.pyx    pyflint.c
acb_poly.pyx    arf.pyx                                fmpq_series.pyx  __init__.py      pyflint.pxd
acb.pyx         dirichlet.pyx                          fmpz_mat.pyx     nmod_mat.pyx     pyflint.pyx
acb_series.pyx  _flint.cpython-38-x86_64-linux-gnu.so  fmpz_mpoly.pyx   nmod_poly.pyx
arb_mat.pyx     _flint.pxd                             fmpz_poly.pyx    nmod.pyx
arb_poly.pyx    fmpq_mat.pyx                           fmpz.pyx         nmod_series.pyx
arb.pyx         fmpq_poly.pyx                          fmpz_series.pyx  __pycache__

Some files e.g. fmpz_series.pyx have no tests of any kind.

@oscarbenjamin
Copy link
Collaborator Author

With the latest cython from git it doesn't seem necessary to patch cython to get the coverage report to work but there are some problems:

Building python-flint is much slower. Maybe this is because cython didn't compile itself or something when installing from local directory... I'll try the latest prerelease to see if that changes it...

There are several build warnings from cython:

warning: src/flint/_flint.pxd:18:0: The 'DEF' statement is deprecated
and will be removed in a future Cython version. Consider using global 
variables, constants, and in-place literals instead. See 
https://github.com/cython/cython/issues/4310

The tests fail immediately with

Traceback (most recent call last):
  File "test/test.py", line 439, in <module>
    sys.stdout.write("test_fmpz..."); test_fmpz(); print("OK")
  File "test/test.py", line 26, in test_fmpz
    assert ltype(s) + rtype(t) == s + t
TypeError: unsupported operand type(s) for +: 'int' and 'flint._flint.fmpz'

@oscarbenjamin
Copy link
Collaborator Author

I get the same results with the prerelease cython==3.0.0a11.

Also I noticed another warning:

src/flint/pyflint.c: In function ‘__pyx_pymod_exec__flint’:
src/flint/pyflint.c:286190:30: note: variable tracking size limit exceeded with -fvar-tracking-assignments, retrying without
 static CYTHON_SMALL_CODE int __pyx_pymod_exec__flint(PyObject *__pyx_pyinit_module)
                              ^

@oscarbenjamin
Copy link
Collaborator Author

The test failure is due to this change in cython:
https://cython.readthedocs.io/en/latest/src/userguide/special_methods.html#arithmetic-methods

Arithmetic operator methods, such as __add__(), used to behave differently from their Python counterparts in Cython 0.x, following the low-level semantics of the C-API slot functions. Since Cython 3.0, they are called in the same way as in Python, including the separate “reversed” versions of these methods (__radd__(), etc.).

Previously, if the first operand could not perform the operation, the same method of the second operand was called, with the operands in the same order. This means that you could not rely on the first parameter of these methods being “self” or being the right type, and you needed to test the types of both operands before deciding what to do.

If backwards compatibility is needed, the normal operator method (__add__, etc.) can still be implemented to support both variants, applying a type check to the arguments. The reversed method (__radd__, etc.) can always be implemented with self as first argument and will be ignored by older Cython versions, whereas Cython 3.x and later will only call the normal method with the expected argument order, and otherwise call the reversed method instead.

I guess __radd__ etc should be included now.

@oscarbenjamin
Copy link
Collaborator Author

I've opened #34 and #35 to look at fixing things for Cython 3.x.

For now this PR works with a patched cython==0.29.32 but is otherwise good so I'll merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants