Opened 10 years ago
Closed 9 years ago
#14076 closed defect (fixed)
Update ChomP to latest upstream version (compilation failure)
Reported by: | Christian Nassau | Owned by: | tbd |
---|---|---|---|
Priority: | major | Milestone: | sage-5.11 |
Component: | packages: optional | Keywords: | |
Cc: | John Palmieri, Travis Scrimshaw, Harald Schilly | Merged in: | sage-5.11.rc0 |
Authors: | Volker Braun | Reviewers: | John Palmieri |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #14595 | Stopgaps: |
Description (last modified by )
Currently (Sage 5.7.beta3) on OpenSuse 12.2
with gcc version 4.7.1 20120723 the CHomP package (version chomp-20100213.p2) can't be installed. This is all fixed in the latest upstream version.
Updated optional spkg: http://boxen.math.washington.edu/home/vbraun/spkg/chomp-20130518.p0.spkg
By the way, this should be added to the optional spkgs, not the experimental ones (and the old one should be removed from the experimental list): there was a vote on this in sage-devel over two years ago to make this change, but I think it never happened.
Apply to the Sage library:
Attachments (2)
Change History (27)
comment:1 Changed 10 years ago by
Cc: | John Palmieri added |
---|
comment:2 Changed 10 years ago by
comment:3 Changed 9 years ago by
Authors: | → Volker Braun |
---|---|
Dependencies: | → #14595 |
Description: | modified (diff) |
Summary: | CHomP compilation failure → Update ChomP to latest upstream version (compilation failure) |
With #14595 this leads to the following doctest failure --- which notation do we prefer for the homology groups?
********************************************************************** File "sage/homology/chain_complex.py", line 708, in sage.homology.chain_complex.ChainComplex.homology Failed example: C_k.homology(generators=true) Expected: {0: [(Z, (1))], 1: [(C2, (1, 0, 0)), (Z, (0, 0, 1))], 2: [(0, ())]} Got: {0: (Z, [(1)]), 1: (Z x C2, [(0, 1, -1), (0, 0, 1)])} **********************************************************************
comment:4 Changed 9 years ago by
Cc: | Travis Scrimshaw added |
---|
comment:5 Changed 9 years ago by
Also, this blows up in your face:
sage: t0 = SimplicialComplex() sage: t0.add_face(('a', 'b')) sage: t0._numeric True
No its not!
sage: t0.homology() --------------------------------------------------------------------------- ValueError Traceback (most recent call last) <ipython-input-27-8061bc7260ca> in <module>() ----> 1 t0.homology() /home/vbraun/opt/sage-5.10.beta5/local/lib/python2.7/site-packages/sage/homology/cell_complex.pyc in homology(self, dim, **kwds) 521 if 'subcomplex' in kwds: 522 del kwds['subcomplex'] --> 523 H = homsimpl(self, subcomplex, **kwds) 524 # now pick off the requested dimensions 525 if H: /home/vbraun/opt/sage-5.10.beta5/local/lib/python2.7/site-packages/sage/interfaces/chomp.pyc in homsimpl(complex, subcomplex, **kwds) 481 if (isinstance(complex, SimplicialComplex) 482 and (subcomplex is None or isinstance(subcomplex, SimplicialComplex))): --> 483 return CHomP()('homsimpl', complex, subcomplex=subcomplex, **kwds) 484 else: 485 raise TypeError, "Complex and/or subcomplex are not simplicial complexes." /home/vbraun/opt/sage-5.10.beta5/local/lib/python2.7/site-packages/sage/interfaces/chomp.pyc in __call__(self, program, complex, subcomplex, **kwds) 209 elif simplicial: 210 m = re.search(r'\(([^,]*),', data) --> 211 v = int(m.group(1)) 212 subcomplex = SimplicialComplex([[v]]) 213 else: ValueError: invalid literal for int() with base 10: "'a'"
Its a bit of a variant of #14578: caching for mutable objects is fragile. Its difficult to get right, and even if you do you'll break it when you make changes in the future.
The right thing to do is to make cell complexes immutable, and add_face
/ remove_face
return new cell complexes. With an add_faces
analogue to add multiple faces in one go to avoid performance issues. Bonus: the cell complexes can then be parents for chains. Thoughts?
comment:6 Changed 9 years ago by
I certainly agree that the add_face
method is overly complicated now. I wouldn't mind making everything immutable. Anyone else have any objections?
Regarding "With #14595 this leads to the following doctest failure --- which notation do we prefer for the homology groups?" I have a slight preference for
{0: (Z, [(1)]), 1: (Z x C2, [(0, 1, -1), (0, 0, 1)])}
but I don't feel strongly about it.
comment:7 Changed 9 years ago by
By the way, the immutability issue was discussed here, where there were objections to making complexes immutable.
comment:8 Changed 9 years ago by
If we are going to do this, we should also make remove_faces
be able to remove multiple faces as well. I will be okay with them being immutable.
comment:9 Changed 9 years ago by
I have the same installation problem as originally reported with SuSE when using Sage 5.10 on OS X. Sage ships with its own version of gcc on this platform (currently 4.7.3), so any OS X user with Sage 5.10 will experience this issue as well if they try to install Chomp.
I tried the updated Chomp package at the above link and it installed and works fine.
comment:10 Changed 9 years ago by
Status: | new → needs_review |
---|
Ok I added a patch that fixes the trivial doctest failure and un-caches the self._numeric
. Once the simplicial complexes are unique we can easily cache the _numeric
check, though its only linear in the number of vertices so its highly unlikely to be a performance bottleneck.
Ready for review...
comment:11 Changed 9 years ago by
The answers given by CHomP and Sage without CHomP for the homology generators of C_k
are different: without CHomP:
{0: [(Z, (1))], 1: [(C2, (1, 0, 0)), (Z, (0, 0, 1))], 2: [(0, ())]}
With CHomP:
{0: (Z, [(1)]), 1: (Z x C2, [(0, 1, -1), (0, 0, 1)])}
As far as I can tell, the non-CHomP answer is correct: (1,0,0) generates a copy of C_2
, because (2,0,0) is in the image of the differential d_2
, while (0,0,2) is not in the image, so (0,0,1) does not generate a copy of C_2
. I haven't yet investigated whether this is a problem in CHomP (which I doubt) or a problem in how we are translating CHomP's answers (more likely).
comment:12 Changed 9 years ago by
By the way, in order to make simplicial complexes unique, do we need to deprecate add_face
and remove_face
(since we need to change their functionality)? Any ideas for names for methods to replace them, which add/remove faces but return new complexes?
comment:13 Changed 9 years ago by
Making simplicial complexes immutable isn't something that can be done gradually, so code that relies on them being mutable will necessarily break.
Having said that, I never was very fond of the add/remove_face
. To me, that implies that you have already a higher-dimensional cell to which you are adding a boundary cell. I would prefer add_simplex
(for simplicial complexes) or maybe just add_cell
(same interface for all cell complexes).
I agree that components of the torsion generator are reversed, will have a look at that.
comment:14 Changed 9 years ago by
ChomP actually uses (0,1,-1)
as Z_2
generator and (0,0,1)
as free generator, we are mixing up the free and torsion part:
sage: c.homology(verbose=True, generators=True) Chain complex over Integer Ring Popen called with arguments ['homchain', '/home/vbraun/.sage/temp/laptop/5244/tmp_ZZ8Q1j', '', '-g/home/vbraun/.sage/temp/laptop/5244/tmp_zVMC6I'] CHomP output: HOMCHAIN, ver. 2.08, 10/07/04. Copyright (C) 1997-2013 by Pawel Pilarczyk. This is free software. No warranty. Consult 'license.txt' for details. [Tech info: chain 24, addr 8, intgr 2.] Reading a chain complex from '/home/vbraun/.sage/temp/laptop/5244/tmp_ZZ8Q1j'... Time used so far: 0.00 sec (0.000 min). The ring of coefficients is the ring of integers. Computing the homology of the chain complex... Reducing D_2: 0 + 2 reductions made. Reducing D_1: 0 + 0 reductions made. H_0 = Z H_1 = Z_2 + Z Writing the homology generators of CX to '/home/vbraun/.sage/temp/laptop/5244/tmp_zVMC6I'... Total time used: 0.00 sec (0.000 min). Thank you for using this software. We appreciate your business. End of CHomP output Generators: ; This file contains the generators of the homology module ; of the chain complex read from the file '/home/vbraun/.sage/temp/laptop/5244/tmp_ZZ8Q1j'. [H_0] a1 [H_1] a2 - a3 a3 [H_2] ('0', 'Z') dimension = 0, number of factors = 1, invariants = [0] raw generators: ; This file contains the generators of the homology module ; of the chain complex read from the file '/home/vbraun/.sage/temp/laptop/5244/tmp_ZZ8Q1j'. [H_0] a1 [H_1] a2 - a3 a3 [H_2] ('1', 'Z_2 + Z') dimension = 1, number of factors = 2, invariants = [2, 0] raw generators: ; This file contains the generators of the homology module ; of the chain complex read from the file '/home/vbraun/.sage/temp/laptop/5244/tmp_ZZ8Q1j'. [H_0] a1 [H_1] a2 - a3 a3 [H_2] {0: (Z, [(1)]), 1: (Z x C2, [(0, 1, -1), (0, 0, 1)])}
comment:16 Changed 9 years ago by
Reviewers: | → John Palmieri |
---|
Okay, here's a patch which sorts the list of generators so it matches the sorted list of abelian group invariants. It also fixes one bug (a missing argument in a call to suspension
), and also modifies two of your new doctests: on OS X vs. linux, sorted(['a', 'b', 123])
yield different results (on sage.math, 123 is first; on OS X, 123 is last).
comment:17 Changed 9 years ago by
Description: | modified (diff) |
---|
By the way, I'm happy with your patch.
comment:18 Changed 9 years ago by
Description: | modified (diff) |
---|
comment:19 Changed 9 years ago by
Cc: | Harald Schilly added |
---|---|
Description: | modified (diff) |
Status: | needs_review → positive_review |
Doctests pass for me, looks good!
comment:20 Changed 9 years ago by
spkg landed in optional on the server/mirrors and the older experimental one is gone (thx for the history lesson, at least i never got notified about this)
comment:21 Changed 9 years ago by
Thanks, Harald. Just to be clear, my message was purely informational, absolutely no criticism was intended. I assumed that no one had told you before, and this upgrade to the spkg was a good time to take care of it.
comment:24 Changed 9 years ago by
There are a few very minor issues with the spkg that I didn't catch. Follow-up ticket: #14872.
comment:25 Changed 9 years ago by
Merged in: | → sage-5.11.rc0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
The required fixes seem to be included in the current version 2013-01-02 of the CHomP package, so an update should fix this issue.