sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.3k stars 447 forks source link

Kenzo interface is broken #27554

Closed jdemeyer closed 5 years ago

jdemeyer commented 5 years ago

Seems like a missing import...

sage -t --long src/sage/interfaces/kenzo.py
**********************************************************************
File "src/sage/interfaces/kenzo.py", line 157, in sage.interfaces.kenzo.EilenbergMacLaneSpace
Failed example:
    f3 = EilenbergMacLaneSpace(AdditiveAbelianGroup([2]), 3)   # optional - kenzo
Exception raised:
    Traceback (most recent call last):
      File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.kenzo.EilenbergMacLaneSpace[3]>", line 1, in <module>
        f3 = EilenbergMacLaneSpace(AdditiveAbelianGroup([Integer(2)]), Integer(3))   # optional - kenzo
      File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage/interfaces/kenzo.py", line 164, in EilenbergMacLaneSpace
        elif G in CommutativeAdditiveGroups() and G.cardinality() == 2:
    NameError: global name 'CommutativeAdditiveGroups' is not defined
**********************************************************************
File "src/sage/interfaces/kenzo.py", line 158, in sage.interfaces.kenzo.EilenbergMacLaneSpace
Failed example:
    [f3.homology(i) for i in range(8)]                         # optional - kenzo
Exception raised:
    Traceback (most recent call last):
      File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/worker/sage-patchbot/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 1095, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.kenzo.EilenbergMacLaneSpace[4]>", line 1, in <module>
        [f3.homology(i) for i in range(Integer(8))]                         # optional - kenzo
    NameError: name 'f3' is not defined
**********************************************************************

A problem with the instalation using different partitions has been solved upstream. We include a new version of the package here. The new tarball is in https://github.com/miguelmarco/kenzo/releases/download/1.1.7-r3/kenzo-1.1.7-r3.tar.gz

Upstream: Reported upstream. No feedback yet.

CC: @miguelmarco @nbruin

Component: interfaces: optional

Author: Miguel Marco

Branch/Commit: 4cbeb6f

Reviewer: Dima Pasechnik

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

dimpase commented 5 years ago
comment:1

It doesn't even install for me:

...
kenzo-1.1.7-r1] ;;; Emitting code for CHML-CLSS.
[kenzo-1.1.7-r1] ;;; Finished compiling /mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/kenzo-1.1.7-r1/src/src/smith.lisp.
[kenzo-1.1.7-r1] ;;;
[kenzo-1.1.7-r1] ;;; Warning: Lisp compilation had style-warnings while compiling #<cl-source-file "kenzo" "src" "smith">
[kenzo-1.1.7-r1] ("smith" "whitehead" "cl-space-efhm" "cs-twisted-products" "serre" "k-pi-1" "classifying-spaces" "lp-space-efhm" "ls-twisted-products" "loop-spaces" ...)
[kenzo-1.1.7-r1] > 
[kenzo-1.1.7-r1] Condition of type: SIMPLE-FILE-ERROR
[kenzo-1.1.7-r1] Unable to rename file #P"/home/dima/.cache/common-lisp/ecl-16.1.2-unknown-linux-x64/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/kenzo-1.1.7-r1/src/kenzo--all-systems.fasb" to #P"/mnt/opt/Sage/sage-dev/local/var/tmp/sage/build/kenzo-1.1.7-r1/src/kenzo--all-systems.fasb".
[kenzo-1.1.7-r1] C library error: "Invalid cross-device link"
[kenzo-1.1.7-r1] 
[kenzo-1.1.7-r1] Available restarts:
[kenzo-1.1.7-r1] 
[kenzo-1.1.7-r1] 1. (RESTART-TOPLEVEL) Go back to Top-Level REPL.
[kenzo-1.1.7-r1] 
[kenzo-1.1.7-r1] Broken at SI:BYTECODES. [Evaluation of: (ASDF/BUNDLE:MAKE-BUILD :KENZO :TYPE :FASL :MONOLITHIC T :MOVE-HERE #P".")]
[kenzo-1.1.7-r1] >> 
[kenzo-1.1.7-r1] moving kenzo--all-systems.fasb to  /mnt/opt/Sage/sage-dev/local/lib/ecl/kenzo.fas
[kenzo-1.1.7-r1] mv: cannot stat 'kenzo--all-systems.fasb': No such file or directory
[kenzo-1.1.7-r1] Error installing Kenzo.
[kenzo-1.1.7-r1] 
dimpase commented 5 years ago
comment:2

seems like the problem is in copying between partitions, and this has already been fixed for Maxima on #8645

dimpase commented 5 years ago
comment:3

Perhaps Nils knows how to do this properly, as no traces of the fix from #8645 can be found, and the fix itself does not seem to work - perhaps it's outdated.

jhpalmieri commented 5 years ago
comment:4

I think this should fix the doctest, but I don't know about Dima's installation problem.

diff --git a/src/sage/interfaces/kenzo.py b/src/sage/interfaces/kenzo.py
index cc24ed3f0c..f0b4a1aafb 100644
--- a/src/sage/interfaces/kenzo.py
+++ b/src/sage/interfaces/kenzo.py
@@ -36,6 +36,7 @@ from sage.structure.sage_object import SageObject
 from sage.homology.homology_group import HomologyGroup
 from sage.rings.integer_ring import ZZ
 from sage.groups.additive_abelian.additive_abelian_group import AdditiveAbelianGroup
+from sage.categories.commutative_additive_groups import CommutativeAdditiveGroups

 from sage.libs.ecl import EclObject, ecl_eval, EclListIterator
dimpase commented 5 years ago

Upstream: Reported upstream. No feedback yet.

dimpase commented 5 years ago
comment:5

The following patch makes it work for me

--- a/compile.lisp
+++ b/compile.lisp
@@ -5,6 +5,7 @@

 (require :asdf)
+(asdf:disable-output-translations)
 (push #P"./" asdf:*central-registry*)
 (require :kenzo)
 (asdf:make-build :kenzo :type :fasl :monolithic t :move-here #P".")

(yes, I RTFM'd for a bit... :-)

I've opened https://github.com/miguelmarco/kenzo/pull/1 to put this fix upstream

miguelmarco commented 5 years ago
comment:6

I did a new upstream release with Dima's patch:

https://github.com/miguelmarco/kenzo/releases/tag/1.1.7-r2

dimpase commented 5 years ago
comment:7

I'm a bit unsure whether, given that Kenzo is now a Sage package, about Kenzo examples in src/sage/homology/simplicial_set_examples.py.

Should there at least be a script (or Lisp code) available there to re-create these files?

miguelmarco commented 5 years ago
comment:8

Replying to @dimpase:

I'm a bit unsure whether, given that Kenzo is now a Sage package, about Kenzo examples in src/sage/homology/simplicial_set_examples.py.

Should there at least be a script (or Lisp code) available there to re-create these files?

Our plan with Ana Romero and her student is to keep working on a more complete interface between Kenzo and Sage. The long term idea is to be able to seamlessly translate simplicial sets back and forth. That includes writing some specific Kenzo code to be able to create simplicial sets from the kind of information that Sage uses to define them, and to output the kind of information that Sage would need.

Considering that, I would wait until we have that ready to redesign the interface used in those examples.

miguelmarco commented 5 years ago

Branch: u/mmarco/kenzo_interface_is_broken

miguelmarco commented 5 years ago
comment:10

I added the missing import, and also made a version bump of the kenzo package, to include Dima's patch (tarball is in https://github.com/miguelmarco/kenzo/archive/1.1.7-r2.tar.gz ).

Can you please test it?


New commits:

1383821Add import and version bump
miguelmarco commented 5 years ago

Commit: 1383821

dimpase commented 5 years ago
comment:11

OK, this works. By the way, I'm not sure all the contents of the tarball should be there, e.g. archive/ and jupyter notebooks in examples.

You can create a slimmed down tarball and add to the release.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

4cbeb6fVersion bump to slimmed down tarball
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 1383821 to 4cbeb6f

miguelmarco commented 5 years ago
comment:13

Replying to @dimpase:

OK, this works. By the way, I'm not sure all the contents of the tarball should be there, e.g. archive/ and jupyter notebooks in examples.

You can create a slimmed down tarball and add to the release.

Done! The tarball is at https://github.com/miguelmarco/kenzo/archive/1.1.7-r3.tar.gz

dimpase commented 5 years ago
comment:14

The tarball for Sage need not be the same as what you automatically get from github. On the releases page there is "Edit" button to the right of each release, clicking on it allows you to add more "assets", e.g. a tarball - properly named, so that one can just wget it without a need to rename.

miguelmarco commented 5 years ago
comment:15

Replying to @dimpase:

The tarball for Sage need not be the same as what you automatically get from github. On the releases page there is "Edit" button to the right of each release, clicking on it allows you to add more "assets", e.g. a tarball - properly named, so that one can just wget it without a need to rename.

I know, but that kind of goes against the idea of version control. I do prefear to have everything fully synced.

dimpase commented 5 years ago
comment:16

Well, in fact sometimes we do have issues with automatically created, by github, tarballs, as they are not stable, they may be affected by changes in the git repo!

https://www.google.com/search?q=github+tarballs+changing

dimpase commented 5 years ago
comment:17

I'd rather use something like git submodules/subtrees/subwhatever for source Sage packages, in fact. This is IMHO safer than tarballs, generally speaking.

miguelmarco commented 5 years ago
comment:18

But the tarballs corresponding to releases shouldn't change at all, right?

dimpase commented 5 years ago
comment:19

Replying to @miguelmarco:

But the tarballs corresponding to releases shouldn't change at all, right?

I am trying to say that the automatically generated by github tarballs are not stable, they may change overtime, and should be avoided. On the other hand, if you add to a github release an "asset" tarball, it will stay intact.

dimpase commented 5 years ago
comment:20

just to explain, I did the following, as an example: https://github.com/dimpase/kenzo/releases/tag/test

there you see https://github.com/dimpase/kenzo/releases/download/test/kenzo-test.tar.gz which is the copy of what I got after clicking on https://github.com/dimpase/kenzo/archive/test.tar.gz and uploading back using "Edit" button. While the latter tarball might change overtime, the former would not, and it also has the correct name.

miguelmarco commented 5 years ago
comment:21

I see, thanks for the hint.

I just added that to the release. The link is now https://github.com/miguelmarco/kenzo/releases/download/1.1.7-r3/kenzo-1.1.7-r3.tar.gz

dimpase commented 5 years ago
comment:22

OK, can you update the branch to reflect this - or do checksum remain the same?

miguelmarco commented 5 years ago
comment:23

The checksums have not changed

dimpase commented 5 years ago
comment:24

Ok, please provide a link to the tarballs in the ticket description, as we always do for package updates.

miguelmarco commented 5 years ago

Description changed:

--- 
+++ 
@@ -32,3 +32,7 @@
     NameError: name 'f3' is not defined
 **********************************************************************

+ + + +A problem with the instalation using different partitions has been solved upstream. We include a new version of the package here. The new tarball is in https://github.com/miguelmarco/kenzo/releases/download/1.1.7-r3/kenzo-1.1.7-r3.tar.gz

dimpase commented 5 years ago

Reviewer: Dima Pasechnik

dimpase commented 5 years ago
comment:26

feel free to set it to positive review, and provide author name.

miguelmarco commented 5 years ago

Author: Miguel Marco

vbraun commented 5 years ago

Changed branch from u/mmarco/kenzo_interface_is_broken to 4cbeb6f