fix symbolic/pynac.pyx doctests #18257

rwst · 2015-04-20T07:54:04Z

Several doctests in symbolic/pynac.pyx iterate over range(get_ginac_serial(), get_ginac_serial()+100). You guess it (sigh), 100 is arbitrary and, right now with some new tickets that introduce new functions, it has become too small, leading to unrelated doctest fails in symbolic/pynac.pyx. This ticket should make sure that this does not happen again.

CC: @kcrisman

Component: symbolics

Author: Ralf Stephan

Branch/Commit: 52ccd47

Reviewer: Karl-Dieter Crisman

Issue created by migration from https://trac.sagemath.org/ticket/18257

The text was updated successfully, but these errors were encountered:

kcrisman · 2015-05-22T12:48:55Z

comment:1

I agree that this is worth fixing permanently, but unless there is an obvious fix we shouldn't let this hold up adding more symbolic functions.

rwst · 2015-05-25T08:37:52Z

comment:2

Replying to @kcrisman:

I agree that this is worth fixing permanently, but unless there is an obvious fix we shouldn't let this hold up adding more symbolic functions.

This is not really difficult.

The central database is a dictionary globally defined in symbolic/function.pyx named sfunction_serial_dict. It has pairs with key of type unsigned and values of Function objects (which can be GinacFunctions or BuiltinFunctions). Accordingly this database is filled with all known Pynac functions (which have the GinacFunction façade) first and then those defined via BuiltinFunction. The unsigned key is the serial number which, on each new function registration, is incremented by one, namely in GiNaC::function::register_new()(https://github.com/pynac/pynac/blob/master/ginac/function.cpp#L1445) by just adding to the Pynac registry and reporting its size minus one. After the GiNaC functions have been added, the global GINAC_FN_SERIAL is set which is then accessed via get_ginac_serial() to get the start key of the Sage defined functions.

There is a problem if ever a function is deregistered. Can this happen? No mechanism in Pynac exists for this at the moment, so it's not a practical consideration.

So the solution is easy: instead of simply adding 100 to the start point, we try all keys from the start key to the latest key used---this need not be a static variable set on registration. Since functions are never deregistered the latest is, again, just the size of the GiNaC registry minus one. This can be had from a call to g_registered_functions().size() in symbolic/pynac.pyx.

cdef get_fn_serial_c():
    return g_registered_functions().size()

def get_fn_serial():
    return get_fn_serial_c()

sage: from sage.symbolic.pynac import get_fn_serial
sage: get_fn_serial()
127
sage: from sage.symbolic.pynac import get_ginac_serial
sage: get_ginac_serial()
40
sage: from sage.symbolic.function import get_sfunction_from_serial
sage: for i in range(get_ginac_serial(), get_fn_serial()):
    print get_sfunction_from_serial(i)
... // prints all known functions
sage: print get_sfunction_from_serial(get_fn_serial()+1)
None

rwst · 2015-05-25T09:08:45Z

comment:3

As they say the first thing that goes down the drain is the planning. The above solution does not account for user defined Python functions that do not inherit from BuiltinFunction or Function.

rwst · 2015-05-25T13:37:41Z

Branch: u/rws/fix_symbolic_pynac_pyx_doctests

rwst · 2015-05-25T13:38:55Z

Commit: 13ba99d

rwst · 2015-05-25T13:38:55Z

comment:5

No, that was an off-by-one error. Please review.

New commits:

`13ba99d`	`18257: fix symbolic/pynac.pyx doctests`

kcrisman · 2015-05-28T15:14:24Z

comment:6

Trivial - duplicate lines in test

+        sage: from sage.symbolic.pynac import get_fn_serial
+        sage: from sage.symbolic.function import get_sfunction_from_serial
+        sage: from sage.symbolic.pynac import get_fn_serial

This patch seems reasonable. What was comment:3 about? I sense that it was a red herring, but if it could be useful for test-driving this I would love to try it. I assume that since this isn't actually called in Sage but is really just for doctesting, it shouldn't have any speed implications.

sagetrac-git · 2015-05-29T07:11:35Z

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

`52ccd47`	`18257: fix typo`

sagetrac-git · 2015-05-29T07:11:35Z

Changed commit from 13ba99d to 52ccd47

rwst · 2015-05-29T07:12:28Z

Author: Ralf Stephan

rwst · 2015-05-29T07:13:29Z

comment:9

Replying to @kcrisman:

... What was comment:3 about? I sense that it was a red herring, but if it could be useful for test-driving this I would love to try it. I assume that since this isn't actually called in Sage but is really just for doctesting, it shouldn't have any speed implications.

It's taken care of in the doctests.

kcrisman · 2015-05-29T13:09:35Z

comment:10

I'm not sure why but I still get

sage -t src/sage/symbolic/pynac.pyx
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 512, in sage.symbolic.pynac.py_latex_function_pystring
Failed example:
    get_sfunction_from_serial(i) == foo
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 514, in sage.symbolic.pynac.py_latex_function_pystring
Failed example:
    py_latex_function_pystring(i, (x,y^z))
Expected:
    'my args are: x, y^z'
Got:
    '\\mathrm{bar}\\left(x, y^{z}\\right)'
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 603, in sage.symbolic.pynac.py_print_fderivative_for_doctests
Failed example:
    get_sfunction_from_serial(i) == foo
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 605, in sage.symbolic.pynac.py_print_fderivative_for_doctests
Failed example:
    py_print_fderivative(i, (0, 1, 0, 1), (x, y^z))
Expected:
    D[0, 1, 0, 1]func_with_args(x, y^z)
Got:
    D[0, 1, 0, 1](foo)(x, y^z)
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 665, in sage.symbolic.pynac.py_latex_fderivative_for_doctests
Failed example:
    get_sfunction_from_serial(i) == foo
Expected:
    True
Got:
    False
**********************************************************************
File "src/sage/symbolic/pynac.pyx", line 667, in sage.symbolic.pynac.py_latex_fderivative_for_doctests
Failed example:
    py_latex_fderivative(i, (0, 1, 0, 1), (x, y^z))
Expected:
    D[0, 1, 0, 1]func_with_args(x, y^z)
Got:
    D[0, 1, 0, 1]\left(\mathrm{bar}\right)\left(x, y^{z}\right)
**********************************************************************

when I have both this ticket and #15024 together. Did I do something wrong? (Note: this is based off of 6.8.beta1, if it matters.)

rwst · 2015-05-29T15:05:54Z

comment:11

Cannot confirm in a first attempt (I branched from #18257, merged #15024 with git trac pull, then merged develop).

rwst · 2015-05-29T15:10:09Z

comment:12

Same branch does work with pynac-0.3.9 (in case you left that installed and that was the difference between us).

rwst · 2015-05-29T15:12:50Z

comment:13

Your line numbers also don't match #18257.

kcrisman · 2015-05-29T18:53:22Z

comment:14

Okay, I'll trash the branches I made locally and try again - may not be immediately, I'm at a conference.

kcrisman · 2015-06-01T14:18:40Z

Reviewer: Karl-Dieter Crisman

kcrisman · 2015-06-01T14:18:40Z

comment:15

I must have either not merged this one or perhaps in the wrong order or something... Anyway, all systems are go, and as expected #15024 fails without this and works with it. Great!

rwst · 2015-06-01T14:36:06Z

comment:16

Hold on. I have a necessary addition because of a commit in Pynac where I removed the registry entries for some defunct functions which makes two of the doctest here fail. This would fit in nicely here.

sagetrac-git · 2015-06-01T14:48:59Z

Changed commit from 52ccd47 to 557badf

sagetrac-git · 2015-06-01T14:48:59Z

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

`557badf`	`adjust doctest because of upstream changes`

rwst · 2015-06-01T14:50:28Z

comment:18

Could you please have a last quick look?

kcrisman · 2015-06-01T14:57:52Z

comment:19

I don't understand why this fails, though? (I mean it shouldn't currently, nothing failed for me.) Surely it would be better to put that commit with the next Pynac upgrade ticket? Easier for the reviewer to test, then (i.e. first upgrade Pynac, run tests (fail), then add branch, watch tests pass).

rwst · 2015-06-01T15:21:59Z

Changed branch from u/rws/fix_symbolic_pynac_pyx_doctests to u/rws/18257

rwst · 2015-06-01T15:22:30Z

Changed commit from 557badf to 52ccd47

vbraun · 2015-06-02T20:18:15Z

Changed branch from u/rws/18257 to 52ccd47

rwst added this to the sage-6.7 milestone Apr 20, 2015

rwst added c: symbolics labels Apr 20, 2015

kcrisman added p: minor / 4 and removed p: major / 3 labels May 22, 2015

rwst added p: major / 3 and removed p: minor / 4 labels May 25, 2015

rwst modified the milestones: sage-6.7, sage-6.8 May 25, 2015

kcrisman removed the s: needs review label Jun 1, 2015

kcrisman added the s: positive review label Jun 1, 2015

rwst added s: needs work and removed s: positive review labels Jun 1, 2015

rwst added s: needs review and removed s: needs work labels Jun 1, 2015

rwst added s: positive review and removed s: needs review labels Jun 1, 2015

vbraun removed the s: positive review label Jun 2, 2015

vbraun closed this as completed in 3ad7e1f Jun 2, 2015

eviatarbach mentioned this issue Jul 12, 2016

Add Hankel functions and make spherical Bessel and Hankel functions symbolic #15024

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix symbolic/pynac.pyx doctests #18257

fix symbolic/pynac.pyx doctests #18257

rwst commented Apr 20, 2015

kcrisman commented May 22, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

kcrisman commented May 28, 2015

sagetrac-git mannequin commented May 29, 2015

sagetrac-git mannequin commented May 29, 2015

rwst commented May 29, 2015

rwst commented May 29, 2015

kcrisman commented May 29, 2015

rwst commented May 29, 2015

rwst commented May 29, 2015

rwst commented May 29, 2015

kcrisman commented May 29, 2015

kcrisman commented Jun 1, 2015

kcrisman commented Jun 1, 2015

rwst commented Jun 1, 2015

sagetrac-git mannequin commented Jun 1, 2015

sagetrac-git mannequin commented Jun 1, 2015

rwst commented Jun 1, 2015

kcrisman commented Jun 1, 2015

rwst commented Jun 1, 2015

rwst commented Jun 1, 2015

vbraun commented Jun 2, 2015

fix symbolic/pynac.pyx doctests #18257

fix symbolic/pynac.pyx doctests #18257

Comments

rwst commented Apr 20, 2015

kcrisman commented May 22, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

rwst commented May 25, 2015

kcrisman commented May 28, 2015

sagetrac-git mannequin commented May 29, 2015

sagetrac-git mannequin commented May 29, 2015

rwst commented May 29, 2015

rwst commented May 29, 2015

kcrisman commented May 29, 2015

rwst commented May 29, 2015

rwst commented May 29, 2015

rwst commented May 29, 2015

kcrisman commented May 29, 2015

kcrisman commented Jun 1, 2015

kcrisman commented Jun 1, 2015

rwst commented Jun 1, 2015

sagetrac-git mannequin commented Jun 1, 2015

sagetrac-git mannequin commented Jun 1, 2015

rwst commented Jun 1, 2015

kcrisman commented Jun 1, 2015

rwst commented Jun 1, 2015

rwst commented Jun 1, 2015

vbraun commented Jun 2, 2015