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

upgrade e-antic to 0.1.7 #29826

Closed
videlec opened this issue Jun 8, 2020 · 31 comments
Closed

upgrade e-antic to 0.1.7 #29826

videlec opened this issue Jun 8, 2020 · 31 comments

Comments

@videlec
Copy link
Contributor

videlec commented Jun 8, 2020

tarball see checksums.ini

This version is compatible with flint 2.6.0.

CC: @mkoeppe

Component: packages: optional

Author: Vincent Delecroix, Matthias Koeppe

Branch: de6b953

Reviewer: Jonathan Kliem, Matthias Koeppe, Dima Pasechnik

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

@videlec videlec added this to the sage-9.2 milestone Jun 8, 2020
@videlec
Copy link
Contributor Author

videlec commented Jun 8, 2020

Author: Vincent Delecroix

@videlec
Copy link
Contributor Author

videlec commented Jun 8, 2020

Branch: u/vdelecroix/e-antic-0.1.6

@videlec
Copy link
Contributor Author

videlec commented Jun 8, 2020

Commit: c05a5eb

@videlec
Copy link
Contributor Author

videlec commented Jun 8, 2020

New commits:

c05a5ebupgrade to e-antic 0.1.6

@kliem
Copy link
Contributor

kliem commented Jun 8, 2020

comment:2

Works for me with and without flint 2.6.0.

Tested the entire geometry folder and the seems to work (with pynormaliz).

Thank you for the quick fix.

@videlec
Copy link
Contributor Author

videlec commented Jun 8, 2020

comment:3

thanks for testing!

@kliem
Copy link
Contributor

kliem commented Jun 8, 2020

comment:4

Btw, tested on debian buster.

The ticket looks fine to me, but I would wait for some confirmation on different systems.

@dimpase
Copy link
Member

dimpase commented Jun 8, 2020

comment:5

so patching flint 2.6.0 was not needed in the end?

@kliem
Copy link
Contributor

kliem commented Jun 8, 2020

comment:6

I just pulled public/packages/flint260, so I guess I did patch.

Either way I'm not sure I entirely understand which patch was needed for what.

@kliem
Copy link
Contributor

kliem commented Jun 8, 2020

comment:7

Replying to @dimpase:

so patching flint 2.6.0 was not needed in the end?

This patch is probably needed, but there is no doctest in sage exposing it.

@videlec
Copy link
Contributor Author

videlec commented Jun 8, 2020

comment:8

I have a bunch of patched version of flint functions in e-antic. There is one more. The patch to flint is not needed for e-antic to work. But the function fmpq_poly_add_fmpq is indeed broken.

@videlec
Copy link
Contributor Author

videlec commented Jun 8, 2020

comment:9

The patch corresponds to this PR flintlib/flint#766

@dimpase
Copy link
Member

dimpase commented Jun 8, 2020

comment:10

Replying to @videlec:

I have a bunch of patched version of flint functions in e-antic. There is one more. The patch to flint is not needed for e-antic to work. But the function fmpq_poly_add_fmpq is indeed broken.

With patches to flint 2.6 all over the place now, it would be good to have all e-antic flint patches upstreamed, so that everything is in one place and hopefully made obsolete by the next flint release.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 10, 2020

Reviewer: Jonathan Kliem, Matthias Koeppe

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 10, 2020

comment:11

Tested on macOS.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 17, 2020

comment:12

Oh - there's 0.1.7 now?

@videlec
Copy link
Contributor Author

videlec commented Jun 17, 2020

comment:13

0.1.7 fixes a bug in printing infinities.

@videlec
Copy link
Contributor Author

videlec commented Jun 17, 2020

comment:14

The bug was only visible on macOS. Did you run the testsuite?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 17, 2020

comment:16

Apparently I didn't.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 17, 2020

Changed author from Vincent Delecroix to Vincent Delecroix, Matthias Koeppe

@mkoeppe

This comment has been minimized.

@mkoeppe mkoeppe changed the title upgrade e-antic to 0.1.6 upgrade e-antic to 0.1.7 Jun 17, 2020
@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 17, 2020

Changed branch from u/vdelecroix/e-antic-0.1.6 to u/mkoeppe/e-antic-0.1.6

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 17, 2020

New commits:

de6b953build/pkgs/e_antic: Update to 0.1.7

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 17, 2020

Changed commit from c05a5eb to de6b953

@dimpase
Copy link
Member

dimpase commented Jun 18, 2020

comment:20

lgtm - after limited testing

hope to catch any more eventual errors on flint update ticket

@dimpase
Copy link
Member

dimpase commented Jun 18, 2020

Changed reviewer from Jonathan Kliem, Matthias Koeppe to Jonathan Kliem, Matthias Koeppe, Dima Pasechnik

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 18, 2020

comment:22

Thanks!

@vbraun
Copy link
Member

vbraun commented Jun 27, 2020

Changed branch from u/mkoeppe/e-antic-0.1.6 to de6b953

@dimpase
Copy link
Member

dimpase commented Jul 31, 2020

Changed commit from de6b953 to none

@dimpase
Copy link
Member

dimpase commented Jul 31, 2020

comment:24

e_antic needs an update to deal with flint 2.6.1, otherwise

[e_antic-0.1.7] In file included from ./e-antic/poly_extra.h:15,
[e_antic-0.1.7]                  from poly_extra/bisection_step_arb.c:12:
[e_antic-0.1.7] ./e-antic/e-antic.h:24:2: error: #error FLINT 2.5.2 or 2.5.3 required
[e_antic-0.1.7]  #error FLINT 2.5.2 or 2.5.3 required
[e_antic-0.1.7]   ^~~~~
[e_antic-0.1.7] In file included from poly_extra/bisection_step_arb.c:12:
[e_antic-0.1.7] ./e-antic/poly_extra.h:267:8: error: static declaration of ‘fmpq_get_d’ follows non-static declaration
[e_antic-0.1.7]  double fmpq_get_d(const fmpq_t q)
[e_antic-0.1.7]         ^~~~~~~~~~
[e_antic-0.1.7] In file included from /mnt/opt/Sage/sage-dev/local/include/flint/fmpz_poly.h:36,
[e_antic-0.1.7]                  from ./e-antic/poly_extra.h:17,
[e_antic-0.1.7]                  from poly_extra/bisection_step_arb.c:12:
[e_antic-0.1.7] /mnt/opt/Sage/sage-dev/local/include/flint/fmpq.h:183:18: note: previous declaration of ‘fmpq_get_d’ was here
[e_antic-0.1.7]  FLINT_DLL double fmpq_get_d(const fmpq_t a);
[e_antic-0.1.7]                   ^~~~~~~~~~
[e_antic-0.1.7] In file included from poly_extra/bisection_step_arb.c:12:
[e_antic-0.1.7] ./e-antic/poly_extra.h:356:2: error: #error "Invalid flint release: e-antic needs flint-2.5.2, flint-2.5.3 or flint-2.6.0"
[e_antic-0.1.7]  #error "Invalid flint release: e-antic needs flint-2.5.2, flint-2.5.3 or flint-2.6.0"
[e_antic-0.1.7]   ^~~~~
[e_antic-0.1.7] make[5]: *** [Makefile:2696: poly_extra/bisection_step_arb.lo] Error 1

I'd say it might be better to rely on a meaningful test to test for a new enough flint, and not just use the version number.

Something akin to what we do in build/pkgs/flint/spkg-configure.m4 - where we check for presence of a function that only appeared in Flint 2.5.0.
The difference between 2.5.0 and 2.5.3 is quite small, just some minor bug fixes, so perhaps you can steal the test from build/pkgs/flint/spkg-configure.m4, and use it, instead of version grepping?

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 31, 2020

comment:25

See #30262 for the e-antic 0.1.8 upgrade (thanks, Vincent, for making the release)

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

5 participants