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

Update ChomP to latest upstream version (compilation failure) #14076

Closed
cnassau opened this issue Feb 7, 2013 · 31 comments
Closed

Update ChomP to latest upstream version (compilation failure) #14076

cnassau opened this issue Feb 7, 2013 · 31 comments

Comments

@cnassau
Copy link

cnassau commented Feb 7, 2013

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:

Depends on #14595

CC: @jhpalmieri @tscrim @haraldschilly

Component: packages: optional

Author: Volker Braun

Reviewer: John Palmieri

Merged: sage-5.11.rc0

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

@cnassau cnassau added this to the sage-5.11 milestone Feb 7, 2013
@cnassau
Copy link
Author

cnassau commented Feb 9, 2013

comment:2

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.

@vbraun
Copy link
Member

vbraun commented May 29, 2013

Author: Volker Braun

@vbraun
Copy link
Member

vbraun commented May 29, 2013

Dependencies: #14595

@vbraun
Copy link
Member

vbraun commented May 29, 2013

comment:3

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)])}
**********************************************************************

@vbraun

This comment has been minimized.

@vbraun vbraun changed the title CHomP compilation failure Update ChomP to latest upstream version (compilation failure) May 29, 2013
@vbraun
Copy link
Member

vbraun commented May 29, 2013

comment:5

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?

@jhpalmieri
Copy link
Member

comment:6

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.

@jhpalmieri
Copy link
Member

comment:7

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

@tscrim
Copy link
Collaborator

tscrim commented Jun 22, 2013

comment:8

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.

@NathanDunfield
Copy link

comment:9

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.

@vbraun
Copy link
Member

vbraun commented Jul 8, 2013

Updated patch

@vbraun
Copy link
Member

vbraun commented Jul 8, 2013

comment:10

Attachment: trac_14076_chomp_update.patch.gz

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

@jhpalmieri
Copy link
Member

comment:11

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

@jhpalmieri
Copy link
Member

comment:12

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?

@vbraun
Copy link
Member

vbraun commented Jul 8, 2013

comment:13

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.

@vbraun
Copy link
Member

vbraun commented Jul 8, 2013

comment:14

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)])}

@jhpalmieri
Copy link
Member

comment:15

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

@jhpalmieri
Copy link
Member

comment:16

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

@jhpalmieri
Copy link
Member

Reviewer: John Palmieri

@jhpalmieri
Copy link
Member

Attachment: trac_14076-more-chomp-fixes.patch.gz

apply second

@jhpalmieri
Copy link
Member

comment:17

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

@jhpalmieri

This comment has been minimized.

@jhpalmieri

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Jul 8, 2013

comment:19

Doctests pass for me, looks good!

@vbraun

This comment has been minimized.

@haraldschilly
Copy link
Member

comment:20

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)

@jhpalmieri
Copy link
Member

comment:21

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.

@haraldschilly
Copy link
Member

comment:22

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

@jhpalmieri
Copy link
Member

comment:23

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

@jhpalmieri
Copy link
Member

comment:24

There are a few very minor issues with the spkg that I didn't catch. Follow-up ticket: #14872.

@jdemeyer
Copy link

Merged: sage-5.11.rc0

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

7 participants