Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#25344 closed task (fixed)

Upgrade cddlib to 0.94j

Reported by: Julian Rüth Owned by:
Priority: major Milestone: sage-8.4
Component: packages: standard Keywords:
Cc: François Bissey, Volker Braun, Timo Kaufmann Merged in:
Authors: Julian Rüth Reviewers: Erik Bray
Report Upstream: N/A Work issues:
Branch: u/saraedum/25344 (Commits, GitHub, GitLab) Commit: 9b7ee991f39918753d3b47ac33edce2fc919fe00
Dependencies: Stopgaps:

Status badges

Description (last modified by Julian Rüth)

and use cddexec instead of cdd_both_reps. cddexec should be faster and is, according to upstream, "done the right way". The change to cddexec and the version upgrade make some numerical issues visible, so some things that worked before are not supported anymore. Additionally, we now require an inexact redundancy-free representation to be convertible back and forth between V and H representation (without loss of vertices/inequalities) before we trust it. Some standard examples, fail this check. I can not really make the call on whether these are bugs in cddlib or numerical issues that could not possibly be fixed (as I do not understand this well enough.) So why this change? Mostly because we do not patch cddlib anymore and just use it as is. It is also faster in some cases (that's what upstream cared most about when the cddlib author heard about our cdd_both_reps,) e.g.,

%timeit polytopes.six_hundred_cell(exact=False)

went from 993ms to 246ms Some other examples see minor slowdowns because of the additional transformation that we compute initially to make sure that the conversion back and forth worked out.

Note that we also don't need the patched random number generator as upstream now uses a portable rng.

Tarball: https://github.com/cddlib/cddlib/releases/download/0.94j/cddlib-0.94j.tar.gz

Attachments (1)

cddlib-0.94j.tar.gz (1.3 MB) - added by Julian Rüth 4 years ago.
The latest cddlib release

Download all attachments as: .zip

Change History (47)

comment:1 Changed 4 years ago by François Bissey

Cc: François Bissey added

comment:2 Changed 4 years ago by Erik Bray

Why wait? If we have the tarball we can do it now, and it will be uploaded to Sage's mirrors.

comment:3 Changed 4 years ago by Julian Rüth

Because I would still like to get your cygwin fixes into upstream.

comment:4 Changed 4 years ago by Erik Bray

Ah, ok.

comment:5 Changed 4 years ago by Julian Rüth

Branch: u/saraedum/25344

comment:6 Changed 4 years ago by git

Commit: 3df595dfb1b407861e699259bde90aa715bb2e31

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

3df595dUpgrade cddlib

comment:7 Changed 4 years ago by git

Commit: 3df595dfb1b407861e699259bde90aa715bb2e314e877117498f1cbd0c8f0cc32539ccbf3c9b1c01

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

4e87711Upgrade cddlib

comment:8 Changed 4 years ago by Julian Rüth

Status: newneeds_review
Work issues: do all doctests pass?

I am running --long doctests for all of Sage currently; everything in geometry/ passes.

comment:9 Changed 4 years ago by Julian Rüth

Description: modified (diff)

comment:10 Changed 4 years ago by Julian Rüth

Description: modified (diff)

comment:11 Changed 4 years ago by Julian Rüth

Work issues: do all doctests pass?do all doctests pass?, the tarball is not final yet

comment:12 Changed 4 years ago by Julian Rüth

Work issues: do all doctests pass?, the tarball is not final yetthe tarball is not final yet

comment:13 Changed 4 years ago by git

Commit: 4e877117498f1cbd0c8f0cc32539ccbf3c9b1c01af9d5be91821223f0f191910a3a8d5f0e38169a0

Branch pushed to git repo; I updated commit sha1. New commits:

14f2c32Fix failing doctests
af9d5beRelax cddlib sanity checks

comment:14 Changed 4 years ago by Julian Rüth

Description: modified (diff)

comment:15 Changed 4 years ago by Julian Rüth

Cc: Volker Braun Timo Kaufmann added

ccing people who previously touched the cddlib integration code.

comment:16 Changed 4 years ago by git

Commit: af9d5be91821223f0f191910a3a8d5f0e38169a0c4048677393f7088d1aa9a2c1392ed95d809d31c

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

b9a9a03Upgrade cddlib
9a649fcFix failing doctests
c404867Relax cddlib sanity checks

comment:17 Changed 4 years ago by Julian Rüth

Work issues: the tarball is not final yetdo all doctests pass?

comment:18 Changed 4 years ago by git

Commit: c4048677393f7088d1aa9a2c1392ed95d809d31c32987a6aa197b274cb4bf550a8fa5567250924a5

Branch pushed to git repo; I updated commit sha1. New commits:

32987a6Update hashes with actual release hashes

comment:19 Changed 4 years ago by Julian Rüth

Summary: Upgrade cddlib to 0.94j (once it's released)Upgrade cddlib to 0.94j

New commits:

32987a6Update hashes with actual release hashes

comment:20 Changed 4 years ago by Erik Bray

I assume this has the Cygwin fixes in it now?

comment:21 Changed 4 years ago by Julian Rüth

Yes.

comment:22 Changed 4 years ago by git

Commit: 32987a6aa197b274cb4bf550a8fa5567250924a5c4b0f10c614c7bfa5efaa8ab3908ccb687e992e4

Branch pushed to git repo; I updated commit sha1. New commits:

c4b0f10Merge branch '25344' into HEAD

comment:23 Changed 4 years ago by Julian Rüth

Work issues: do all doctests pass?

comment:24 Changed 4 years ago by Julian Rüth

All tests pass.

comment:25 Changed 4 years ago by Erik Bray

Description: modified (diff)
Reviewers: Erik Bray
Status: needs_reviewpositive_review

comment:26 Changed 4 years ago by Timo Kaufmann

Is the GitHub (https://github.com/cddlib/cddlib) repo considered the official upstream (with approval of the original author) now or is that a fork?

Last edited 4 years ago by Timo Kaufmann (previous) (diff)

comment:28 Changed 4 years ago by Julian Rüth

It has the author's approval. However, the author is considering to abandon it if the recent acquisiton of github turns the platform the wrong way.

comment:29 Changed 4 years ago by François Bissey

In any case I think we would prefer to have a link to an official tarball rather than an attachment - especially for distros (well me in Gentoo in particular).

comment:30 Changed 4 years ago by Timo Kaufmann

Thanks for your work, that greatly simplifies packaging :) Every patch replaced by something upstream supported is a win.

Could you maybe ask them to make a note on the cddlib homepage? That would help people that stumble there and also make it seem a bit more legit.

comment:31 Changed 4 years ago by Julian Rüth

Description: modified (diff)

Sorry, I wasn't aware of the details of the SPKG upgrade process. Actually, the checksums are for the tarball on github.

Changed 4 years ago by Julian Rüth

Attachment: cddlib-0.94j.tar.gz added

The latest cddlib release

comment:32 in reply to:  30 Changed 4 years ago by Julian Rüth

Replying to gh-timokau:

Thanks for your work, that greatly simplifies packaging :) Every patch replaced by something upstream supported is a win.

Could you maybe ask them to make a note on the cddlib homepage? That would help people that stumble there and also make it seem a bit more legit.

This is now https://github.com/cddlib/cddlib/issues/18.

comment:33 Changed 4 years ago by François Bissey

A link is enough. If we are going to use the tarball https://github.com/cddlib/cddlib/releases/download/0.94j/cddlib-0.94j.tar.gz then spkg-src is not needed. As it been prepared with make sdist (and then why not sdist-bzip2 or whatever the option is)? If so there shouldn't be any intermediate autotools files to clean.

comment:34 in reply to:  33 ; Changed 4 years ago by Julian Rüth

Replying to fbissey:

A link is enough. If we are going to use the tarball https://github.com/cddlib/cddlib/releases/download/0.94j/cddlib-0.94j.tar.gz then spkg-src is not needed. As it been prepared with make sdist (and then why not sdist-bzip2 or whatever the option is)? If so there shouldn't be any intermediate autotools files to clean.

The tarball came out of make distcheck. I don't understand the question about the intermediate autotools files.

comment:35 in reply to:  34 Changed 4 years ago by François Bissey

Replying to saraedum:

Replying to fbissey:

A link is enough. If we are going to use the tarball https://github.com/cddlib/cddlib/releases/download/0.94j/cddlib-0.94j.tar.gz then spkg-src is not needed. As it been prepared with make sdist (and then why not sdist-bzip2 or whatever the option is)? If so there shouldn't be any intermediate autotools files to clean.

The tarball came out of make distcheck. I don't understand the question about the intermediate autotools files.

Good enough I guess, although I never used that target. The question about intermediate autotools files is related to spkg-src, I looked at what it is supposed to do. spkg-src is usually to take upstream and then produce a new tarball for distribution in sage for whatever reasons. That particular spkg-src mostly does cleaning. But if we use the tarball produced by make distcheck we don't need it at all and can remove it.

comment:36 Changed 4 years ago by Vincent Delecroix

Milestone: sage-8.3sage-8.4

update milestone 8.3 -> 8.4

comment:37 Changed 4 years ago by git

Commit: c4b0f10c614c7bfa5efaa8ab3908ccb687e992e49b7ee991f39918753d3b47ac33edce2fc919fe00
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

9b7ee99Merge remote-tracking branch 'trac/develop' into 25344

comment:38 Changed 4 years ago by Julian Rüth

Status: needs_reviewpositive_review

trivial merge

comment:39 Changed 4 years ago by Volker Braun

Don't modify positively reviewed tickets unless you have a reason. There was no merge conflict, so no need to do anything. I'll ignore the superfluous merge.

comment:40 Changed 4 years ago by Volker Braun

New commits:

9b7ee99Merge remote-tracking branch 'trac/develop' into 25344

comment:41 Changed 4 years ago by Volker Braun

Resolution: fixed
Status: positive_reviewclosed

comment:42 Changed 4 years ago by Volker Braun

New commits:

9b7ee99Merge remote-tracking branch 'trac/develop' into 25344

comment:43 Changed 4 years ago by Volker Braun

The trac plugin automatically updates the commit field to the branch head, even though this is not the version that was merged (=c4b0f10c614c7bfa5efaa8ab3908ccb687e992e4).

comment:44 Changed 4 years ago by Julian Rüth

I needed a clean patch against 8.3 for Debian and Conda distribution so I decided to push the merge so that I can say in the packaging scripts that this is actually the patch exactly as it is going into upstream.

I am sorry if this caused some trouble for you. I didn't see the harm of merging in develop. What is the problem other than some noise here on the ticket?

comment:45 Changed 4 years ago by Volker Braun

The problem is that I find that the branch has changed after running the buildbot, when I'm about to close the ticket....

comment:46 Changed 4 years ago by Julian Rüth

I see. Sorry for that then.

Note: See TracTickets for help on using tickets.