Opened 3 years ago
Closed 3 years ago
#27554 closed defect (fixed)
Kenzo interface is broken
Reported by:  jdemeyer  Owned by:  

Priority:  blocker  Milestone:  sage8.8 
Component:  interfaces: optional  Keywords:  
Cc:  mmarco, nbruin  Merged in:  
Authors:  Miguel Marco  Reviewers:  Dima Pasechnik 
Report Upstream:  Reported upstream. No feedback yet.  Work issues:  
Branch:  4cbeb6f (Commits, GitHub, GitLab)  Commit:  4cbeb6f2815246ef7550eea05f787cbe3084633d 
Dependencies:  Stopgaps: 
Description (last modified by )
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/sagepatchbot/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/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/sagepatchbot/local/lib/python2.7/sitepackages/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/sagepatchbot/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 671, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/worker/sagepatchbot/local/lib/python2.7/sitepackages/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.7r3/kenzo1.1.7r3.tar.gz
Change History (28)
comment:1 Changed 3 years ago by
comment:2 Changed 3 years ago by
seems like the problem is in copying between partitions, and this has already been fixed for Maxima on #8645
comment:3 Changed 3 years ago by
 Cc nbruin added
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.
comment:4 Changed 3 years ago by
I think this should fix the doctest, but I don't know about Dima's installation problem.

src/sage/interfaces/kenzo.py
diff git a/src/sage/interfaces/kenzo.py b/src/sage/interfaces/kenzo.py index cc24ed3f0c..f0b4a1aafb 100644
a b from sage.structure.sage_object import SageObject 36 36 from sage.homology.homology_group import HomologyGroup 37 37 from sage.rings.integer_ring import ZZ 38 38 from sage.groups.additive_abelian.additive_abelian_group import AdditiveAbelianGroup 39 from sage.categories.commutative_additive_groups import CommutativeAdditiveGroups 39 40 40 41 from sage.libs.ecl import EclObject, ecl_eval, EclListIterator 41 42
comment:5 Changed 3 years ago by
 Report Upstream changed from N/A to Reported upstream. No feedback yet.
The following patch makes it work for me

compile.lisp
a b 5 5 6 6 7 7 (require :asdf) 8 (asdf:disableoutputtranslations) 8 9 (push #P"./" asdf:*centralregistry*) 9 10 (require :kenzo) 10 11 (asdf:makebuild :kenzo :type :fasl :monolithic t :movehere #P".")
(yes, I RTFM'd for a bit... :)
I've opened https://github.com/miguelmarco/kenzo/pull/1 to put this fix upstream
comment:6 Changed 3 years ago by
I did a new upstream release with Dima's patch:
comment:7 followup: ↓ 8 Changed 3 years ago by
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 recreate these files?
comment:8 in reply to: ↑ 7 Changed 3 years ago by
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 recreate 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.
comment:9 Changed 3 years ago by
 Branch set to u/mmarco/kenzo_interface_is_broken
comment:10 Changed 3 years ago by
 Commit set to 1383821b3dcaf55f525c6c52194d21c13f4ae9f3
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.7r2.tar.gz ).
Can you please test it?
New commits:
1383821  Add import and version bump

comment:11 followup: ↓ 13 Changed 3 years ago by
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.
comment:12 Changed 3 years ago by
 Commit changed from 1383821b3dcaf55f525c6c52194d21c13f4ae9f3 to 4cbeb6f2815246ef7550eea05f787cbe3084633d
Branch pushed to git repo; I updated commit sha1. New commits:
4cbeb6f  Version bump to slimmed down tarball

comment:13 in reply to: ↑ 11 Changed 3 years ago by
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.7r3.tar.gz
comment:14 followup: ↓ 15 Changed 3 years ago by
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.
comment:15 in reply to: ↑ 14 Changed 3 years ago by
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.
comment:16 Changed 3 years ago by
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!
comment:17 Changed 3 years ago by
I'd rather use something like git submodules/subtrees/subwhatever for source Sage packages, in fact. This is IMHO safer than tarballs, generally speaking.
comment:18 followup: ↓ 19 Changed 3 years ago by
But the tarballs corresponding to releases shouldn't change at all, right?
comment:19 in reply to: ↑ 18 Changed 3 years ago by
Replying to mmarco:
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.
comment:20 Changed 3 years ago by
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/kenzotest.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.
comment:21 Changed 3 years ago by
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.7r3/kenzo1.1.7r3.tar.gz
comment:22 Changed 3 years ago by
OK, can you update the branch to reflect this  or do checksum remain the same?
comment:23 Changed 3 years ago by
The checksums have not changed
comment:24 Changed 3 years ago by
Ok, please provide a link to the tarballs in the ticket description, as we always do for package updates.
comment:25 Changed 3 years ago by
 Description modified (diff)
comment:26 Changed 3 years ago by
 Reviewers set to Dima Pasechnik
 Status changed from new to needs_review
feel free to set it to positive review, and provide author name.
comment:27 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:28 Changed 3 years ago by
 Branch changed from u/mmarco/kenzo_interface_is_broken to 4cbeb6f2815246ef7550eea05f787cbe3084633d
 Resolution set to fixed
 Status changed from positive_review to closed
It doesn't even install for me: