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:

Status badges

Description (last modified by Volker Braun)

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)

trac_14076_chomp_update.patch (7.2 KB) - added by Volker Braun 9 years ago.
Updated patch
trac_14076-more-chomp-fixes.patch (3.1 KB) - added by John Palmieri 9 years ago.
apply second

Download all attachments as: .zip

Change History (27)

comment:1 Changed 10 years ago by John Palmieri

Cc: John Palmieri added

comment:2 Changed 10 years ago by Christian Nassau

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.

comment:3 Changed 9 years ago by Volker Braun

Authors: Volker Braun
Dependencies: #14595
Description: modified (diff)
Summary: CHomP compilation failureUpdate 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 Volker Braun

Cc: Travis Scrimshaw added

comment:5 Changed 9 years ago by Volker Braun

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 John Palmieri

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 John Palmieri

By the way, the immutability issue was discussed here, where there were objections to making complexes immutable.

Last edited 9 years ago by John Palmieri (previous) (diff)

comment:8 Changed 9 years ago by Travis Scrimshaw

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 Nathan Dunfield

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.

Changed 9 years ago by Volker Braun

Updated patch

comment:10 Changed 9 years ago by Volker Braun

Status: newneeds_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 John Palmieri

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 John Palmieri

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 Volker Braun

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 Volker Braun

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:15 Changed 9 years ago by John Palmieri

Right, I have a patch for this, coming up.

comment:16 Changed 9 years ago by John Palmieri

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).

Changed 9 years ago by John Palmieri

apply second

comment:17 Changed 9 years ago by John Palmieri

Description: modified (diff)

By the way, I'm happy with your patch.

comment:18 Changed 9 years ago by John Palmieri

Description: modified (diff)

comment:19 Changed 9 years ago by Volker Braun

Cc: Harald Schilly added
Description: modified (diff)
Status: needs_reviewpositive_review

Doctests pass for me, looks good!

comment:20 Changed 9 years ago by Harald Schilly

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 John Palmieri

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:22 Changed 9 years ago by Harald Schilly

language barrier alert. i didn't mean it that way ;-)

comment:23 Changed 9 years ago by John Palmieri

(I didn't think so, but I wanted to be clear.)

comment:24 Changed 9 years ago by John Palmieri

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 Jeroen Demeyer

Merged in: sage-5.11.rc0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.