Opened 3 years ago

Last modified 19 months ago

#20739 needs_work enhancement

Make snappy a pip package

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-7.3
Component: packages: optional Keywords: days74, snappy
Cc: culler, mgoerner, dunfield, mmarco, tscrim, slelievre Merged in:
Authors: Vincent Delecroix Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: public/20739 (Commits) Commit: be1b40d99510bbbeeacc60b66fd0fcf8e8a3a03f
Dependencies: Stopgaps:

Description (last modified by slelievre)

Make SnapPy an "official" optional package. With the branch apply you just need to run

sage -i snappy

Sadly, the options such as --user are not transmitted to pip. So for a user wide installation, one still need to use pip version

sage -pip install snappy --user

In a follow-up ticket, we could ensure compatibility of the Sage Link class and the SnapPy link class via some doctests.

Note: a Docker image for Sage + SnapPy + Regina + PHCPack + more is available:

Change History (13)

comment:1 Changed 3 years ago by vdelecroix

  • Branch set to public/20739
  • Commit set to be1b40d99510bbbeeacc60b66fd0fcf8e8a3a03f
  • Description modified (diff)

New commits:

be1b40dTrac 20739: snappy as an "official" package

comment:2 Changed 3 years ago by vdelecroix

  • Description modified (diff)

comment:3 Changed 3 years ago by mmarco

Nice!

How exactly do you imagine the integration with our Link class?

comment:4 Changed 3 years ago by vdelecroix

As a first step, Nathan tought us the sage_link method

sage: 
sage: import snappy
sage: L = snappy.Link('K10n11')
sage: K = L.sage_link()
sage: K.alexander_polynomial()
-t^-3 + 5*t^-2 - 9*t^-1 + 11 - 9*t + 5*t^2 - t^3
sage: L.alexander_polynomial()
t^6 - 5*t^5 + 9*t^4 - 11*t^3 + 9*t^2 - 5*t + 1

And there is a nice conversion from Sage link to SnapPy? handled automatically

sage: B = BraidGroup(8)
sage: K = Link(B([-1, -1, -1, -2, 1, -2, 3, -2, 3]))
sage: L = snappy.Link(K)

This might already help for:

  • double checking the outputs (from both sides)
  • comparing the implementations

To enhance it further we could make work the natural conversion the other way around

sage: L = snappy.Link('K10n11')
sage: Link(L)

comment:5 follow-up: Changed 3 years ago by tscrim

Something that would be good to have upstream is a _sage_ method as that is the standard way to call something back to something Sage knows about. In any case, our Link code should just call the appropriate method in the SnapPy Link (and as a bonus, we can pass strings to SnapPy and convert those back to Sage so the knot names work).

As for passing the --user option, we should talk to Jeroen or Volker about that.

comment:6 in reply to: ↑ 5 Changed 3 years ago by dunfield

Replying to tscrim:

Something that would be good to have upstream is a _sage_ method as that is the standard way to call something back to something Sage knows about.

I just added the _sage_ method to Spherogram, essentially as an alias for the sage_link method mentioned above.

comment:7 Changed 3 years ago by jdemeyer

  • Authors set to Vincent Delecroix
  • Reviewers set to Jeroen Demeyer
  • Status changed from new to needs_review
  • Summary changed from Make snappy an optional package to Make snappy a pip package

comment:8 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:9 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:10 Changed 3 years ago by dunfield

  • Status changed from positive_review to needs_work

I'm concerned that pip is not being passed the flag --no-binary :all: (which is equivalent to the old --no-use-wheel). On OS X, the binary wheel we post on PyPI is built against a "Frameworks" Python rather than the "plain Unix" Python that ships with Sage. I couldn't reproduce this when I tried right now, but I have observed in the past where "sage -pip install snappy" on OS X results in installing the binary wheel from PyPI; this results in a fatal linking error when trying to load the module.

This is not a problem on Linux simply because we currently don't ship binary wheels for Linux --- the standard for that was only recently finalized --- but that will change soon.

comment:11 Changed 3 years ago by dunfield

  • Cc culler mgoerner added

comment:12 Changed 3 years ago by dimpase

Please take note of this SMC issue and my proposal there. The problem is that snappy is also the name of a Python module for the compression library python-snappy, which is used in the very popular machine-learning package tensorflow.

I propose there to use relative imports to avoid this clash.

comment:13 Changed 19 months ago by slelievre

  • Cc slelievre added
  • Description modified (diff)
  • Keywords snappy added

Adding the following note to the ticket description, for reference.

Note: a Docker image for Sage + SnapPy + Regina + PHCPack + more is available:

Note: See TracTickets for help on using tickets.