Track pyx files in citation modules/ reorganize it

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

cProfile does not track c-calls. Google performance tools do so, hand have been wrapped up recently. This integrates the gperftools into sage.misc.citation, reorganizes code for future development, and fixes one issue with globals. Concerning the latter, Cython was used wrongly, in as far as it's globals() are incorrect in the currenct version. The inspect module is the right way to go, guaranteed to work in future versions as well.

CC: @mwhansen @nathanncohen

Component: misc

Author: Martin Raum

Branch/Commit: u/mraum/ticket/16777 @ b423bfe

Reviewer: Volker Braun

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

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

Branch: u/mraum/ticket/16777

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

1bd0d5c	Increase profile frequency to catch all pyx calls.
bc9955c	Correct parsing of gperftools output.
5296d27	Redict stderr of gperftools.
e2c99c1	Decrease profiling frequency and ignore warnings.
920545b	Reorganize names in citation module.
5da28f0	Rename helper functions.
2ae588f	Add doctests.
3d2ba32	Add documentation and doctests.
b1d2e8e	Add documentation, doctests, fix language.
7e9194a	Insert ticket number for deprecation warning.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Commit: 7e9194a

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

Dependencies: 16778

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 7e9194a to 0131486

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

f4bf79a	Decrease profiling frequency and ignore warnings.
c2413e6	Reorganize names in citation module.
c37720c	Rename helper functions.
2f67a8b	Add doctests.
d112736	Add documentation and doctests.
450275b	Add documentation, doctests, fix language.
ff8a926	Insert ticket number for deprecation warning.
e1ea516	Use gprofiler by directly saving to tmp file.
6f72e4b	Add citations module to documentation.
0131486	Fix documentation.

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

Changed dependencies from 16778 to none

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

comment:5

I removed dependence on #16778 by directly loading the profiling output.

[fchapoton]

comment:6

one doctest failing, see patchbot report

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

5d294a4	Fix doctest failure.
87bcd1f	Merge tag '6.3' into citation branch.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 0131486 to 87bcd1f

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

comment:9

I fixed that, and also merged 6.3.

[vbraun]

comment:10

lgtm

[vbraun]

Reviewer: Volker Braun

[vbraun]

Changed branch from u/mraum/ticket/16777 to 87bcd1f

[vbraun]

comment:13

This failed on my desktop with:

sage -t --long src/sage/misc/citation.py
    Killed due to signal 27
**********************************************************************
Tests run before process (pid=15468) failed:
sage: with citations():
      var('y')
      integrate(y^2, y, 0, 1) ## line 15 ##
y
1/3
The computation used the following components.
Access them as a list by calling latest_citations().
    ginac, Maxima
sage: latest_citations() ## line 31 ##
[ginac, Maxima]
sage: record = [] ## line 40 ##
sage: with citations(record):
      K = QuadraticField(-3, 'a') ## line 41 ##
sage: record ## line 43 ##
[GAP, GMP, MPFI, MPFR, NTL]
sage: with citations(record):
      integrate(y^2, y, 0, 1) ## line 45 ##
1/3
sage: record ## line 48 ##
[GAP, GMP, MPFI, MPFR, NTL, ginac, Maxima]
sage: eval_citations("integrate(y^2, y, 0, 10)") ## line 55 ##
[ginac, Maxima]
sage: sig_on_count() ## line 61 ##
0
sage: a = var('a') ## line 83 ##
sage: with citations():
      h = ((a+1)^2).expand() ## line 84 ##
The computation used the following components.
Access them as a list by calling latest_citations().
    ginac, GMP
sage: latest_citations() ## line 89 ##
[ginac, GMP]
sage: sig_on_count() ## line 91 ##
0
sage: R.<x,y,z> = QQ[] ## line 109 ##
sage: I = R.ideal(x^2+y^2, z^2+y) ## line 110 ##
sage: with citations():
      I.primary_decomposition() ## line 111 ##
[Ideal (z^2 + y, x^2 + y^2) of Multivariate Polynomial Ring in x, y, z over Rational Field]
The computation used the following components.
Access them as a list by calling latest_citations().
    Macaulay2, Singular
sage: latest_citations() ## line 117 ##
[Macaulay2, Singular]
sage: sig_on_count() ## line 119 ##
0
sage: s = eval_citations( "integrate(x^2, x)" ); #priming coercion model ## line 221 ##
sage: eval_citations('integrate(x^2, x)') ## line 222 ##

Signal 27 = SIGTTOU? Maybe there is a signal handler race?

[vbraun]

Changed commit from 87bcd1f to none

[vbraun]

Changed branch from 87bcd1f to u/mraum/ticket/16777

[vbraun]

Commit: 87bcd1f

[vbraun]

comment:14

Also, Nathan told me that pprof sometimes dies because we don't stop profiling under certain circumstances. Might be related, though I don't know how...

---

Last 10 new commits:

c37720c	Rename helper functions.
2f67a8b	Add doctests.
d112736	Add documentation and doctests.
450275b	Add documentation, doctests, fix language.
ff8a926	Insert ticket number for deprecation warning.
e1ea516	Use gprofiler by directly saving to tmp file.
6f72e4b	Add citations module to documentation.
0131486	Fix documentation.
5d294a4	Fix doctest failure.
87bcd1f	Merge tag '6.3' into citation branch.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

ddc10e4

Redirect stdout and stderr while profiling.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 87bcd1f to ddc10e4

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

comment:16

I can reproduce this when letting sage -t citation.py run 100 times. I tried redirecting stdout and stderr, but this didn't fix the problem. Any ideas on how to solve this?

[vbraun]

comment:17

Pprof probably mucks with the signals. Assuming that it temporarily changes the signal mask, there would be a window where SIGTTOU would come from a subprocess. Just redirecting stdin/out doesn't change the fact that there is flow control. One might try to disable TOSTOP but I think that'll just leave the race open for other signals. IMHO a better solution would be to (re-)compile Sage with Cython profiling turned on and then use CProfiler.

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

comment:18

I'm happy to change the code so that we don't use gperftools. But I don't think I have the legitimation to change global compilation parameters. I even don't know what to change so that cProfile will include pyx files.

Can you, Volker, deal with the compilation. I'll make necessary changes to the citation system in a separate ticket, if required.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

5150a5c	Revert "Redirect stdout ... while profiling."
3487e5c	Remove support for gperftools in citation system.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from ddc10e4 to 3487e5c

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

comment:21

While this seems subject of ongoing discussion, I have already pushed separate changes -- BibTeX facilities -- to #16854.

[fchapoton]

comment:22

I have made a small commit, to change a few lines to the new docstyle continuation using ....:

---

New commits:

68029f3	Merge branch 'u/mraum/ticket/16777' of ssh://trac.sagemath.org:22/sage into 16777
7964470	trac #16777 using new-style of doctest continuation

[fchapoton]

Changed branch from u/mraum/ticket/16777 to public/ticket/16777

[fchapoton]

Changed commit from 3487e5c to 7964470

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

Changed branch from public/ticket/16777 to u/mraum/ticket/16777

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

comment:24

This implements the approach that was suggested by Volker, reasoning that branch prediction will avoid slow down in critical (e.g. Cython or C) code. The test suite runs, on my computer, 1.3% slower. I have also tested a more critical part, sage/rings, which runs 2.8% slower.

See https://groups.google.com/d/msg/sage-devel/xoPszCcMtns/Y5UYdOr-WVsJ for a discussion. As a remark, I want to say also here: If we want to have citations (which I strongly believe is a question of publication ethics), either we need the profiling approach (which does not interoperate well with the Sage ecosystem) or we have to accept a minor slow down.

[18d65770-dc1e-4813-89a9-4828e4aae4a9]

Changed commit from 7964470 to b423bfe

[fchapoton]

comment:25

Branch does not apply

[jdemeyer]

comment:26

Is anybody still interested in this ticket? Note that the ticket description no longer reflects the code.