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

Interface to runsnake and import_statements #11287

Closed
nthiery opened this issue May 4, 2011 · 46 comments
Closed

Interface to runsnake and import_statements #11287

nthiery opened this issue May 4, 2011 · 46 comments

Comments

@nthiery
Copy link
Contributor

nthiery commented May 4, 2011

This patch adds two developper tools in sage.misc.dev_tools:

  • An interface to the graphical profiler runsnake.
    See e.g. the attached image which shows a graphical profile
    of listing all the elements of SymmetricGroup(3), and highlights
    the current GAP interface issue with select on certain linux
    kernels. See also Interface to Gprof2Dot #11291.

  • A function import_statements:

        sage: import_statements(WeylGroup, lazy_attribute)
        from sage.combinat.root_system.weyl_group import WeylGroup
        from sage.misc.lazy_attribute import lazy_attribute

        sage: import_statements(WeylGroup, lazy_attribute, lazy=True)
        from sage.misc.lazy_import import lazy_import
        lazy_import('sage.combinat.root_system.weyl_group', 'WeylGroup')
        lazy_import('sage.misc.lazy_attribute', 'lazy_attribute')

To achieve this, it also extracts a function get_main_globals out of inject_variables.

Apply: attachment: trac_11287-runsnake-nt.patch

Depends on #11251

CC: @sagetrac-sage-combinat @saliola @anneschilling @novoselt

Component: performance

Keywords: runsnake, prun, profiling, days30

Author: Nicolas M. Thiéry

Reviewer: Franco Saliola, Simon King, Christian Stump

Merged: sage-4.7.2.alpha0

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

@nthiery nthiery added this to the sage-4.7.1 milestone May 4, 2011
@nthiery
Copy link
Contributor Author

nthiery commented May 4, 2011

Attachment: runsnake-screenshot.png

Screen shot of runsnake in action

@nthiery

This comment has been minimized.

@nthiery

This comment has been minimized.

@nthiery

This comment has been minimized.

@sagetrac-nborie
Copy link
Mannequin

sagetrac-nborie mannequin commented May 4, 2011

comment:4

As I use very very heavily runsnake this last mouth, I try your patch as soon as I received the corresponding mail.

Applying the patch (toward the combinat queue), runsnake works well on my machine in sage. (I had to remove my old runsnake function in .sage/init.sage and re-install RunSankeRun vis easy-install).

Ubuntu 04-11 32bits on a MacBook4,1. Not a review, just a test and some info...

+1 to get the feature in Sage!

@simon-king-jena
Copy link
Member

comment:5

Runsnake

Can you give at least a slight hint how to use runsnake? I used easy_install inside a Sage shell, but apparently something (wxPython?) is missing. I thought easy_install is supposed to take care of dependencies.

Also, I think you should give examples how to use the profiler from within Sage. From runsnake's documentation, I guess one does something like

sage: import cProfile
sage: cProfile.run'L=[singular(i)==gap(i)==i+1 for i in range(2000)]'
cProfile.run     cProfile.runctx  
sage: cProfile.runctx('L=[singular(i)==gap(i)==i+1 for i in range(2000)]', globals(),locals(), filename="OpenGLContext.profile" )

but what afterwards?

Does one need to leave Sage, start a Sage shell, and then open OpenGLContext.profile by runsnake OpenGLContext.profile? (That step did not work for me, it seems I need to get wx)

Can one use it without leaving Sage?

import_statements

That tool seems useful. I doubt, however, that it is robust enough. I see that you ask for the __module__ attribute. It is not always present, at least for methods or for extension classes that forgot to initialise the category:

sage: from sage.structure.element import Element
sage: Element.parent.__module__
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/mnt/local/king/SAGE/sage-4.7.alpha5/<ipython console> in <module>()

AttributeError: 'method_descriptor' object has no attribute '__module__'
sage: P.<x,y> = QQ[]
sage: P.__module__
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)

/mnt/local/king/SAGE/broken/devel/sage-main/<ipython console> in <module>()

/mnt/local/king/SAGE/broken/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.Parent.__getattr__ (sage/structure/parent.c:5618)()

/mnt/local/king/SAGE/broken/local/lib/python2.6/site-packages/sage/structure/parent.so in sage.structure.parent.raise_attribute_error (sage/structure/parent.c:2636)()

AttributeError: 'sage.rings.polynomial.multi_polynomial_libsingular.MPolynomialRing_libsingular' object has no attribute '__module__'

Worse, the __module__ attribute can be misleading. Unfortunately, it is particularly misleading if you initialise the category of a parent:

sage: P._is_category_initialized()
False
sage: P._init_category_(Algebras(QQ))
sage: P.__module__
'sage.categories.algebras'
sage: import_statements(P)
from sage.categories.category import CommutativeAlgebras.parent_class

That import statement yields a syntax error.

Also, I recall that sometimes the __module__ attribute is None (but I can't remember a typical example).

I would recommend to use the tools from sage.misc.sageinspect, but only after applying #9976, which considerably extends the capabilities of finding source files (here: sage/rings/polynomial/multi_polynomial_libsingular.pyx), from which one can deduce the import statement.

I am not sure if this is enough to make it "needs work", though.

Cheers,
Simon

@simon-king-jena
Copy link
Member

Changed reviewer from Franco Saliola to Franco Saliola, Simon King

@saliola
Copy link

saliola commented May 4, 2011

comment:6

Hello! I just posted a reviewer's patch that addresses some documentation
issues. The docstrings for the Profiler module needed to be fixed so that they
rendered correctly in the reference manual.

Note that this patch depends on #11251, since you are using the new todo
directive that is implemented in that ticket.

I am willing to give this a positive review, provided my changes are
acceptable.

Let me try to address Simon's comments:

Replying to @simon-king-jena:

Runsnake

Can you give at least a slight hint how to use runsnake?

There is an example on how to use it provided in the
documentation:

EXAMPLES::

    sage: runsnake("list(SymmetricGroup(3))")     # optional - requires runsnake

The expected behaviour is that a window pops up (see the screenshot).

Does one need to leave Sage, start a Sage shell, and then open OpenGLContext.profile by runsnake OpenGLContext.profile? (That step did not work for me, it seems I need to get wx)

Can one use it without leaving Sage?

Yes. One needs only run the function in the example above. This function
initializes the profiler and (doing essentially what you wrote above),
and then launches runsnake.

import_statements

That tool seems useful.

Agreed. It looks very nice.

Worse, the __module__ attribute can be misleading. Unfortunately, it is particularly misleading if you initialise the category of a parent:

This issue is documented in a "todo" block, but this todo block does not
appear in the command-line view of the documentation. So that's probably why
you didn't see it (it does appear if you ask for source
import_statements??). This is not a problem with this ticket though, but
with the todo directive; see #11251.

I would recommend to use the tools from sage.misc.sageinspect, but only after applying #9976, which considerably extends the capabilities of finding source files (here: sage/rings/polynomial/multi_polynomial_libsingular.pyx), from which one can deduce the import statement.

Perhaps this might be a good idea and can address the issue that the
__module__ attribute is not always present. I'll leave it to Nicolas to
comment on this.

@saliola
Copy link

saliola commented May 5, 2011

Changed keywords from runsnake, prun, profiling to runsnake, prun, profiling, days30

@nthiery
Copy link
Contributor Author

nthiery commented May 5, 2011

comment:8

Replying to @saliola:

Hello! I just posted a reviewer's patch that addresses some documentation
issues. The docstrings for the Profiler module needed to be fixed so that they
rendered correctly in the reference manual.
...
I am willing to give this a positive review, provided my changes are
acceptable.

Positive review on your review patch, assuming the buildbot goes green.

I would recommend to use the tools from sage.misc.sageinspect, but only after applying #9976, which considerably extends the capabilities of finding source files (here: sage/rings/polynomial/multi_polynomial_libsingular.pyx), from which one can deduce the import statement.

Perhaps this might be a good idea and can address the issue that the
__module__ attribute is not always present. I'll leave it to
Nicolas to comment on this.

If you know how to improve this on top of your head, please go ahead!
Otherwise, I'll vote for just pushing this as is, and fix it later. It
is annoying if this method is not robust, but not dangerous, and we
want to let it be used to get feedback.

Cheers,
Nicolas

@simon-king-jena
Copy link
Member

comment:9

I thought that the improvements to sageinspect from #9976 would make things robust enough, so that sage_getfile could be used to extract the information on where to import stuff from.

But with or without #9976, we have

sage: P.<x,y> = QQ[]
sage: P??
Error getting source: could not find class definition
...
Docstring [source file open failed]:
    File: sage/rings/polynomial/multi_polynomial_libsingular.pyx (starting
    at line 229)
    
       Construct a multivariate polynomial ring subject to the following
       conditions:
    
       INPUT:
    
       * ``base_ring`` - base ring (must be either GF(q), ZZ, ZZ/nZZ,
            QQ or absolute number field)
    
       * ``n`` - number of variables (must be at least 1)
    
       * ``names`` - names of ring variables, may be string of list/tuple
    
       * ``order`` - term order (default: ``degrevlex``)

The "Error getting source: could not find class definition" is annoying. Note that the doc string above gives the correct location and line number! So, sage.misc._extract_embedded_position should be able to do the job, but it isn't.

I think improving the introspection further should be on a different ticket, on top of #9976.

Cheers,
Simon

@simon-king-jena
Copy link
Member

comment:10

I just opened #11298 (ready for review).

With it, we obtain:

sage: from sage.misc.sageinspect import sage_getsourcelines, sage_getfile
sage: P.<x,y> = QQ[]
sage: P._init_category_(Algebras(QQ))
sage: P.__module__
'sage.categories.algebras'
sage: sage_getfile(P)
'/mnt/local/king/SAGE/sage-4.7.alpha5/devel/sage/sage/rings/polynomial/multi_polynomial_libsingular.pyx'
sage: lines, lineno = sage_getsourcelines(P)
sage: lines[0]
'cdef class MPolynomialRing_libsingular(MPolynomialRing_generic):\n'

It should be easy to extract the module name from the file name (here: sage.rings.polynomial.multi_polynomial_libsingular, not sage.categories.algebras). Moreover, it should be easy to extract the correct class name from the first line of the source code (ClassName, not ClassName_with_category or SomeCategory.parent_class, which would not work in an import statement).

Cheers,
Simon

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented May 5, 2011

comment:11

Ubuntu/Debian RunSnakeRun install

Along with a few missteps, this is what I needed to do to have RunSnakeRun on an Ubuntu Lucid system. Presumably this will work for other Debian-based distributions.

wxPython

Follow instructions at:

http://wiki.wxpython.org/InstallingOnUbuntuOrDebian

Python easy_install setup program, pstats profiling module, RunSnakeRun itself

sudo apt-get install python-setuptools python-profiler
easy_install RunSnakeRun

@rbeezer
Copy link
Mannequin

rbeezer mannequin commented May 5, 2011

comment:12

Replying to @rbeezer:

Ubuntu/Debian RunSnakeRun install, part 2

There are better instructions in the doc string. wxPython is an Ubuntu package (python-wxgtk2.8).

@simon-king-jena
Copy link
Member

comment:13

Replying to @rbeezer:

There are better instructions in the doc string. wxPython is an Ubuntu package (python-wxgtk2.8).

OK, but my personal problem is that I am not root for the computer in my office. So, installing an Ubuntu package seems non-trivial to me.

I tried installing wxpython from sources and also tried the experimental wxpython spkg. In both cases, the problem was that gtk+ has not been found, although it is globally installed. The sysadmin tries to help me, but I don't know if it will work eventually.

@simon-king-jena
Copy link
Member

comment:14

Could it be that the Sage shell messes up? Namely, I downloaded wxPython sources. When I do ./configure in a Python shell, it complains about not finding gtk. If I do it on a usual shell, it works. I am now trying make und make install outside the python shell. I hope that it suffices to have wxpython in the path, i.e., it is not assumed that it was installed in the Sage scope?

Best regards,
Simon

@simon-king-jena
Copy link
Member

comment:15

Replying to @simon-king-jena:

Could it be that the Sage shell messes up? Namely, I downloaded wxPython sources. When I do ./configure in a Python shell,

I meant to write "Sage shell".

@simon-king-jena
Copy link
Member

comment:16

Unfortunately, after doing easy_install RunSnakeRun, things still do not work for me:


sage: runsnake("list(SymmetricGroup(3))")
sage: Traceback (most recent call last):
  File "/mnt/local/king/SAGE/sage-4.7.alpha5/local/bin/runsnake", line 5, in <module>
    from pkg_resources import load_entry_point
  File "/usr/lib/python2.6/dist-packages/pkg_resources.py", line 2671, in <module>
    working_set.require(__requires__)
  File "/usr/lib/python2.6/dist-packages/pkg_resources.py", line 654, in require
    needed = self.resolve(parse_requirements(requirements))
  File "/usr/lib/python2.6/dist-packages/pkg_resources.py", line 552, in resolve
    raise DistributionNotFound(req)
pkg_resources.DistributionNotFound: RunSnakeRun==2.0.1b6

I did not do sudo apt-get install python-setuptools python-profiler, as I am not root. Is that the missing bit?

@simon-king-jena
Copy link
Member

comment:17

PS: If I leave sage, but remain in a sage shell, and try runsnake OpenGLContext.profile then the error says ImportError: No module named wx. So, apparently the problem is that I really need to install wxpython inside a Sage shell, but I can't since the Sage shell can not find stuff that is known to the rest of the world. End of the story. I have a lecture to prepare.

@stumpc5
Copy link
Contributor

stumpc5 commented May 8, 2011

comment:18

Hi, I pushed a review concerning two minor things I ran into:

on the sage command line, spaces do not cause trouble, but the profiler doesn't like them:

sage: runsnake(' 3')
...
IndentationError: unexpected indent (<string>, line 1)

Now, spaces are deleted (or does anyone see problems occurring from that?).

The second is that sage accepts "^", while python doesn't, so these are replaced by "**".

sage: runsnake('QQ(3)^2')
...
RuntimeError: Use ** for exponentiation, not '^', which means xor
in Python, and has the wrong precedence.

You can fold the changes if you like, or remove them otherwise...

Btw: the patch doesn't apply on a plain 4.6.2.

Best, Christian

@simon-king-jena
Copy link
Member

comment:19

Replying to @stumpc5:

The second is that sage accepts "^", while python doesn't, so these are replaced by "**".

What about calling the Sage preparser, instead?

@nthiery
Copy link
Contributor Author

nthiery commented May 9, 2011

comment:20

Thanks Christian for your improvements.

Replying to @simon-king-jena:

Replying to @stumpc5:

The second is that sage accepts "^", while python doesn't, so these are replaced by "**".

What about calling the Sage preparser, instead?

+1.

Christian: I won't be able to work on this before a couple day. Could you give it a shot?

Also, I would tend to only strip spaces before and after the command; otherwise, one might inadvertently screw up spaces in an embedded string.

Cheers,

@nthiery
Copy link
Contributor Author

nthiery commented May 9, 2011

comment:21

MacOS users (Anne!): The runsnake run author was kind enough to add a workaround in runsnake to make it work out of the box on MacOS. He would like to get it tested. Could you try out the attached devel version of runsnake, and see if:

    tar zxvf runsnakerun-2.0.1-devel.tgz
    cd runsnakerun-2.0.1-devel
    sudo python setup.py install
    runsnake

does the job for you?

Anne: you may want to first remove the current runsnake scripts from your machine; if I recall correctly, they are in /usr/local/bin/runsnake and runsnake32bit.

@saliola
Copy link

saliola commented May 12, 2011

comment:34

Note that my patch also removes a line of documentation: the globals variable is no longer used by the runsnake command.

@nthiery
Copy link
Contributor Author

nthiery commented May 13, 2011

comment:35

Replying to @saliola:

Note that my patch also removes a line of documentation: the globals variable is no longer used by the runsnake command.

Thanks for the fix. I folded your patch, and am about to set a positive review.

Thanks everybody!

@jdemeyer
Copy link

jdemeyer commented Jun 1, 2011

comment:36

Please change the commit message of the patch.

@nthiery
Copy link
Contributor Author

nthiery commented Jun 1, 2011

Attachment: trac_11287-runsnake-nt.patch.gz

@nthiery
Copy link
Contributor Author

nthiery commented Jun 1, 2011

comment:37

Replying to @jdemeyer:

Please change the commit message of the patch.

Oops, done.

I also removed the dependency on #11108 (which is already merged) since this confuses the buildbot.

@nthiery
Copy link
Contributor Author

nthiery commented Jun 1, 2011

Changed dependencies from #11108 to none

@jdemeyer
Copy link

jdemeyer commented Jun 1, 2011

comment:38

There is a problem with the documentation:

dochtml.log:/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.1.alpha3/local/lib/python2.6/site-packages/sage/misc/dev_tools.py:docstring of sage.misc.dev_tools.import_statements:14: (ERROR/3) Unknown directive type "todo".
dochtml.log:/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.1.alpha3/local/lib/python2.6/site-packages/sage/misc/profiler.py:docstring of sage.misc.profiler.Profiler:27: (ERROR/3) Unknown directive type "todo".

@nthiery
Copy link
Contributor Author

nthiery commented Jun 1, 2011

Dependencies: #11251

@nthiery
Copy link
Contributor Author

nthiery commented Jun 1, 2011

comment:39

Replying to @jdemeyer:

There is a problem with the documentation:

dochtml.log:/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.1.alpha3/local/lib/python2.6/site-packages/sage/misc/dev_tools.py:docstring of sage.misc.dev_tools.import_statements:14: (ERROR/3) Unknown directive type "todo".
dochtml.log:/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.1.alpha3/local/lib/python2.6/site-packages/sage/misc/profiler.py:docstring of sage.misc.profiler.Profiler:27: (ERROR/3) Unknown directive type "todo".

Ah shoot. This directive is implemented by #11251 which needs review. Ok, I added this in the dependency list, and am going to ping #11251.

Thanks for the report ...

@jdemeyer jdemeyer removed this from the sage-4.7.1 milestone Jun 14, 2011
@jdemeyer
Copy link

Merged: sage-4.7.2.alpha0

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

No branches or pull requests

6 participants