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

kenzo package and interface #27188

Closed
miguelmarco opened this issue Jan 31, 2019 · 70 comments
Closed

kenzo package and interface #27188

miguelmarco opened this issue Jan 31, 2019 · 70 comments

Comments

@miguelmarco
Copy link
Contributor

Add a kenzo optional package, and the corresponding interface.

The source file is taken from https://github.com/miguelmarco/kenzo, which is a small fork by Miguel Marco to include some utility scripts from https://github.com/gheber/kenzo, which is in turn a fork of the original source code modified to be compatible with other lisps (in particular it works on ecl).

Tarball (to be renamed kenzo-1.1.7-r1.tar.gz):

(Miguel Marco is maintaining a fork for the moment, this version includes a script to compile kenzo into a .fas file).

The interface uses the ecl interface, and provides a wrapper for some kenzo objects (we hope to extend it in the future).

CC: @jhpalmieri @slel @dimpase

Component: interfaces

Keywords: upgrade, kenzo, topology

Author: Miguel Marco, Ana Romero

Branch: 4a164a8

Reviewer: Travis Scrimshaw

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

@miguelmarco miguelmarco added this to the sage-8.7 milestone Jan 31, 2019
@miguelmarco
Copy link
Contributor Author

Changed keywords from none to kenzo topology

@miguelmarco

This comment has been minimized.

@miguelmarco
Copy link
Contributor Author

Author: Miguel Marco, Ana Romero

@miguelmarco
Copy link
Contributor Author

Branch: u/mmarco/kenzo_package_and_interface

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 31, 2019

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

b54179aAdd optional tag to doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 31, 2019

Commit: b54179a

@jhpalmieri
Copy link
Member

comment:5

This is exciting to see. Can we get an interface between Kenzo and Sage's implementation of simplicial sets? A crude version is in sage.homology.simplicial_set_examples.simplicial_data_from_kenzo_output, which was written to handle the files in src/ext/kenzo. But doing it in lisp or Python rather than just parsing strings would be much better.

@miguelmarco
Copy link
Contributor Author

comment:6

That is the long term idea we had in mind for starting this; but we still haven't decided the design.

Maybe include an attribute for each Sage simplicial set that is a reference to the corresponding kenzo object (via this interface)?

Right now we have just coded the bare minimum kenzo functionality, but for that task maybe we should add support for explicit list of faces and maps.

@miguelmarco
Copy link
Contributor Author

comment:7

I just noticed that the code I pushed in this branch fails when trying to load it. It works ok in my install of Sage based on python3, but fails in the standard python2. Since we don't know how much time will take until python3 makes it into standard sage installs, I will try to find a way to import kenzo inside sage's ecl that works also for python2.

In the meantime, you can test this branch using a python3 based sage, following the instructions in https://groups.google.com/forum/#!topic/sage-devel/uEHBKekJ_Cw

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2019

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

76e6ed3Load asdf before requiring it to prevent problem with python2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2019

Changed commit from b54179a to 76e6ed3

@miguelmarco
Copy link
Contributor Author

comment:9

I just pushed a one liner that prevents the error with python2 (at least in my system). Please test it.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2019

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

ad33c8aMake fix specific for python2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 1, 2019

Changed commit from 76e6ed3 to ad33c8a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 2, 2019

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

a3f051bCompile into .fas file

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 2, 2019

Changed commit from ad33c8a to a3f051b

@miguelmarco

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 2, 2019

Changed commit from a3f051b to aae2671

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 2, 2019

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

aae2671Switch to forked upstream with utility scripts

@miguelmarco
Copy link
Contributor Author

comment:14

Switched to another upstream source. It is a fork I made to be able to include some utility scripts that we might need.

For the moment, it includes a script to compile all kenzo in a self-contained .fasl file, that we can easily load without problems both from python2 and python3.

I think it is pretty much ready to be tested.

@miguelmarco

This comment has been minimized.

@slel

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Feb 3, 2019

comment:17

we could add your scripts to @gheber's repo, provided they are sufficiently generic; after all I already did a bit of work on it:
https://github.com/gheber/kenzo/graphs/contributors :-)

@miguelmarco
Copy link
Contributor Author

comment:18

Replying to @dimpase:

we could add your scripts to @gheber's repo, provided they are sufficiently generic; after all I already did a bit of work on it:
https://github.com/gheber/kenzo/graphs/contributors :-)

We discussed it with gheber and Sergerarert in gheber/kenzo#129 . We finally agreed to fork it to work in whatever we need to add (we might need to add some extra functions to allow the creation of Kenzo objects from the corresponding Sage SimplicialSets counterparts). Then maybe, when we have something stable, we can consider merging back.

@miguelmarco
Copy link
Contributor Author

comment:19

I think this can be reviewed now. We still have to adapt the SimplicialSets module, and maybe write some kenzo code, but that would be better done in future tickets.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 27, 2019

Changed commit from 12b6026 to b95f759

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 27, 2019

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

b95f759Minor doctest fixes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 27, 2019

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

b78241fCapital K for Eilenberg-MacLane docstring

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 27, 2019

Changed commit from b95f759 to b78241f

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 27, 2019

Changed commit from b78241f to d9f8d4a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Feb 27, 2019

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

d9f8d4aMissing terminal points in docstrings

@miguelmarco
Copy link
Contributor Author

comment:42

Thanks for the thorough review!

Is it ok if I set it to positive review?

@tscrim
Copy link
Collaborator

tscrim commented Feb 27, 2019

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Feb 27, 2019

comment:43

You definitely should have put me as reviewer, but it would have been maybe slightly better to let me have one final lookover. However, as I mentioned I will let the little doc stuff slide to avoid over-nitpicking. ;)

@slel
Copy link
Member

slel commented Feb 27, 2019

comment:44

Adding "upgrade" to the keywords so the release manager knows there is a tarball to upload.

What is our convention for renaming the tarball? Do we need version numbers to not contain dashes?

@slel

This comment has been minimized.

@slel
Copy link
Member

slel commented Feb 27, 2019

Changed keywords from kenzo topology to upgrade, kenzo, topology

@tscrim
Copy link
Collaborator

tscrim commented Feb 27, 2019

comment:45

Replying to @slel:

What is our convention for renaming the tarball? Do we need version numbers to not contain dashes?

They can contain dashes, underscores, etc.; e.g.:

lidia-2.3.0+latte-patches-2014-10-04.tar.gz
jmol-14.6.1_2016.07.11.tar.bz2

I don't think we have any specific conventions for tarball names in general other than date formats yyyymmdd for things that do not have version numbers or a sha1.

@slel
Copy link
Member

slel commented Feb 28, 2019

comment:46

Right, I misremembered that. Maybe our package names can't contain dashes,
so the first dash in a tarball name ends the package name and starts the version
number. That might be why we sometimes rename some-package-name
to some_package_name. No problem with kenzo in that respect.

@slel

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Mar 1, 2019

comment:47
sage -t --long /usr/lib64/python2.7/site-packages/sage/interfaces/kenzo.py
**********************************************************************
File "/usr/lib64/python2.7/site-packages/sage/interfaces/kenzo.py", line 153, in sage.interfaces.kenzo.EilenbergMacLaneSpace
Failed example:
    from sage.interfaces.kenzo import EilenbergMacLaneSpace
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/lib64/python2.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.kenzo.EilenbergMacLaneSpace[0]>", line 1, in <module>
        from sage.interfaces.kenzo import EilenbergMacLaneSpace
      File "/usr/lib64/python2.7/site-packages/sage/interfaces/kenzo.py", line 53, in <module>
        ecl_eval("(require :kenzo)")
      File "sage/libs/ecl.pyx", line 1326, in sage.libs.ecl.ecl_eval (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python2_7/build/cythonized/sage/libs/ecl.c:10127)
        cpdef EclObject ecl_eval(str s):
      File "sage/libs/ecl.pyx", line 1341, in sage.libs.ecl.ecl_eval (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python2_7/build/cythonized/sage/libs/ecl.c:10066)
        o=ecl_safe_eval(o)
      File "sage/libs/ecl.pyx", line 350, in sage.libs.ecl.ecl_safe_eval (/dev/shm/portage/sci-mathematics/sage-9999/work/sage-9999/src-python2_7/build/cythonized/sage/libs/ecl.c:5124)
        raise RuntimeError("ECL says: {}".format(
    RuntimeError: ECL says: Module error: Don't know how to REQUIRE KENZO.
**********************************************************************

on a machine without the kenzo package installed. That import line needs to be marked # optional - kenzo as well.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2019

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

4a164a8Add optional tag to doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Mar 1, 2019

Changed commit from d9f8d4a to 4a164a8

@vbraun
Copy link
Member

vbraun commented Mar 3, 2019

Changed branch from u/mmarco/kenzo_package_and_interface to 4a164a8

@jdemeyer
Copy link

Changed commit from 4a164a8 to none

@jdemeyer
Copy link

comment:51

Did anybody ever test that this actually passes doctests? See #27554.

@tscrim
Copy link
Collaborator

tscrim commented Mar 26, 2019

comment:52

I think I might have only run it without the package installed...sorry about that.

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

8 participants