Skip to content

Commit

Permalink
Trac #23162: Give a warning when using citation.get_systems() without…
Browse files Browse the repository at this point in the history
… Cython profiling

`sage.misc.citation.get_systems()` is implemented by using `cProfile` to
look at which modules implement the functions called when executing the
code.

The problem is that this is totally unreliable when Cython is compiled
without profiling support (which is the default). This doctest
{{{
sage: from sage.misc.citation import get_systems
sage: get_systems('((x+1)^2).expand()')
['ginac']
}}}
only works because `Expression.expand()` is called by Python instead of
Cython. If that call would be inside some other Cython code, then
Python's profiler would not detect it:
{{{
sage: cython('def callexpand(x): return x.expand()')
sage: from sage.misc.citation import get_systems
sage: get_systems('callexpand(((x+1)^2))')
[]
}}}

There is a problem because #22747 will "break" profiling even further as
even the top-level call of `Expression.expand()` would not be detected
as something to be entered in the profiler.

So here I propose simply to give a warning whenever `get_systems()` is
used when profiling was not enabled.

URL: https://trac.sagemath.org/23162
Reported by: jdemeyer
Ticket author(s): Jeroen Demeyer
Reviewer(s): Sébastien Labbé
  • Loading branch information
Release Manager authored and vbraun committed Jun 15, 2017
2 parents 7938484 + ae120c5 commit 317f0d8
Showing 1 changed file with 46 additions and 6 deletions.
52 changes: 46 additions & 6 deletions src/sage/misc/citation.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,29 @@ systems['PolyBoRi'] = ['sage.rings.polynomial.pbori']

def get_systems(cmd):
"""
Returns a list of the systems used in running the command
cmd. Note that the results can sometimes include systems
that did not actually contribute to the computation. Due
to caching and the inability to follow all C calls, it
Returns a list of the systems used in running the command ``cmd``.
Note that the results can sometimes include systems that did not
actually contribute to the computation. Due to caching, it
could miss some dependencies as well.
INPUT:
- ``cmd`` - a string to run
.. WARNING::
In order to properly support Cython code, this requires that
Sage was compiled with the environment variable
``SAGE_PROFILE=yes``. If this was not the case, a warning will
be given when calling this function.
EXAMPLES::
sage: from sage.misc.citation import get_systems
sage: s = get_systems('integrate(x^2, x)'); #priming coercion model
sage: get_systems('print("hello")') # random (may print warning)
[]
sage: integrate(x^2, x) # Priming coercion model
1/3*x^3
sage: get_systems('integrate(x^2, x)')
['ginac', 'Maxima']
sage: R.<x,y,z> = QQ[]
Expand All @@ -76,6 +85,13 @@ def get_systems(cmd):
"""
import cProfile, pstats, re

if not cython_profile_enabled():
from warnings import warn
warn("get_systems() requires Cython profiling to be enabled, "
"otherwise the results will be very unreliable. "
"Rebuild Sage with the environment variable 'SAGE_PROFILE=yes' "
"to enable profiling.")

if not isinstance(cmd, basestring):
raise TypeError("command must be a string")

Expand All @@ -91,7 +107,7 @@ def get_systems(cmd):
strings = [a[0].replace(SAGE_ROOT, "") + " " + a[2] for a in stats.stats.keys()]

#Remove trivial functions
bad_res = [re.compile(r'is_.*Element')]
bad_res = [re.compile(r'is_.*Element'), re.compile("is_[a-z_]*_type")]
for bad_re in bad_res:
i = 0
while i < len(strings):
Expand All @@ -106,3 +122,27 @@ def get_systems(cmd):
if any([(r in s) or (r.replace('.','/') in s) for r in systems[system] for s in strings]):
systems_used.append(system)
return systems_used


cdef extern from *:
int CYTHON_PROFILE """
#ifdef CYTHON_PROFILE
CYTHON_PROFILE
#else
0
#endif
"""

cpdef inline bint cython_profile_enabled():
"""
Return whether Cython profiling is enabled.
EXAMPLES::
sage: from sage.misc.citation import cython_profile_enabled
sage: cython_profile_enabled() # random
False
sage: type(cython_profile_enabled()) is bool
True
"""
return CYTHON_PROFILE

0 comments on commit 317f0d8

Please sign in to comment.