Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#29826 closed enhancement (fixed)

upgrade e-antic to 0.1.7

Reported by: Vincent Delecroix Owned by:
Priority: major Milestone: sage-9.2
Component: packages: optional Keywords:
Cc: Matthias Köppe Merged in:
Authors: Vincent Delecroix, Matthias Koeppe Reviewers: Jonathan Kliem, Matthias Koeppe, Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: de6b953 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Matthias Köppe)

tarball see checksums.ini

This version is compatible with flint 2.6.0.

Change History (25)

comment:1 Changed 2 years ago by Vincent Delecroix

Authors: Vincent Delecroix
Branch: u/vdelecroix/e-antic-0.1.6
Commit: c05a5ebdb803f6fe64339f5cfe64a28cf56cbd18
Status: newneeds_review

New commits:

c05a5ebupgrade to e-antic 0.1.6

comment:2 Changed 2 years ago by gh-kliem

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.

comment:3 Changed 2 years ago by Vincent Delecroix

thanks for testing!

comment:4 Changed 2 years ago by gh-kliem

Btw, tested on debian buster.

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

comment:5 Changed 2 years ago by Dima Pasechnik

so patching flint 2.6.0 was not needed in the end?

comment:6 Changed 2 years ago by gh-kliem

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.

comment:7 in reply to:  5 Changed 2 years ago by gh-kliem

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.

comment:8 Changed 2 years ago by Vincent Delecroix

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.

comment:9 Changed 2 years ago by Vincent Delecroix

The patch corresponds to this PR https://github.com/wbhart/flint2/pull/766

comment:10 in reply to:  8 Changed 2 years ago by Dima Pasechnik

Replying to vdelecroix:

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.

comment:11 Changed 2 years ago by Matthias Köppe

Reviewers: Jonathan Kliem, Matthias Koeppe
Status: needs_reviewpositive_review

Tested on macOS.

comment:12 Changed 2 years ago by Matthias Köppe

Oh - there's 0.1.7 now?

comment:13 Changed 2 years ago by Vincent Delecroix

0.1.7 fixes a bug in printing infinities.

comment:14 Changed 2 years ago by Vincent Delecroix

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

comment:15 Changed 2 years ago by Matthias Köppe

Status: positive_reviewneeds_work

comment:16 Changed 2 years ago by Matthias Köppe

Apparently I didn't.

comment:17 Changed 2 years ago by Matthias Köppe

Authors: Vincent DelecroixVincent Delecroix, Matthias Koeppe
Description: modified (diff)
Summary: upgrade e-antic to 0.1.6upgrade e-antic to 0.1.7

comment:18 Changed 2 years ago by Matthias Köppe

Branch: u/vdelecroix/e-antic-0.1.6u/mkoeppe/e-antic-0.1.6

comment:19 Changed 2 years ago by Matthias Köppe

Commit: c05a5ebdb803f6fe64339f5cfe64a28cf56cbd18de6b953d7fa948915bad895392d702d07fddc4b2
Status: needs_workneeds_review

New commits:

de6b953build/pkgs/e_antic: Update to 0.1.7

comment:20 Changed 2 years ago by Dima Pasechnik

Status: needs_reviewpositive_review

lgtm - after limited testing

hope to catch any more eventual errors on flint update ticket

comment:21 Changed 2 years ago by Dima Pasechnik

Reviewers: Jonathan Kliem, Matthias KoeppeJonathan Kliem, Matthias Koeppe, Dima Pasechnik

comment:22 Changed 2 years ago by Matthias Köppe

Thanks!

comment:23 Changed 2 years ago by Volker Braun

Branch: u/mkoeppe/e-antic-0.1.6de6b953d7fa948915bad895392d702d07fddc4b2
Resolution: fixed
Status: positive_reviewclosed

comment:24 Changed 2 years ago by Dima Pasechnik

Commit: de6b953d7fa948915bad895392d702d07fddc4b2

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?

comment:25 Changed 2 years ago by Matthias Köppe

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

Note: See TracTickets for help on using tickets.