Opened 23 months ago

Closed 7 months ago

#31325 closed enhancement (fixed)

Upgrade ecm to 7.0.5

Reported by: Matthias Köppe Owned by:
Priority: critical Milestone: sage-9.7
Component: packages: standard Keywords:
Cc: Paul Zimmermann, Dima Pasechnik, François Bissey, Gonzalo Tornaría Merged in:
Authors: Matthias Koeppe Reviewers: Dima Pasechnik
Report Upstream: N/A Work issues:
Branch: 6a7e098 (Commits, GitHub, GitLab) Commit: 6a7e098ffa40ae04057404c0f7fa7635e3e45a8b
Dependencies: Stopgaps:

Status badges

Description (last modified by Dima Pasechnik)

(from #30600)

official 7.0.5 tarball: see checksums.ini

Change History (44)

comment:1 Changed 23 months ago by Paul Zimmermann

warning, 7.0.5 is not yet released! And the release number might change...

comment:2 Changed 21 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Sage development has entered the release candidate phase for 9.3. Setting a new milestone for this ticket based on a cursory review of ticket status, priority, and last modification date.

comment:3 Changed 17 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

comment:4 Changed 15 months ago by Matthias Köppe

Milestone: sage-9.5sage-pending

comment:5 Changed 8 months ago by Matthias Köppe

Description: modified (diff)

comment:6 Changed 8 months ago by Matthias Köppe

Branch: u/mkoeppe/upgrade_ecm_to_7_0_5

comment:7 Changed 8 months ago by Matthias Köppe

Commit: 39e07273d3d8c49dbec053f08d9a600f4f30b412
Milestone: sage-pendingsage-9.7

New commits:

39e0727build/pkgs/ecm: Update to 7.0.5-dev, drop patch

comment:8 Changed 8 months ago by Matthias Köppe

[ecm-7.0.5-dev] gcc -DHAVE_CONFIG_H -I.  -DOUTSIDE_LIBECM   -g -march=native -g -O2 -fPIC -c -o ecm-main.o `test -f 'main.c' || echo './'`main.c
[ecm-7.0.5-dev] main.c:55:10: fatal error: 'torsions.h' file not found
[ecm-7.0.5-dev] #include "torsions.h" /* to benefit from more torsion groups */
[ecm-7.0.5-dev]          ^~~~~~~~~~~~
[ecm-7.0.5-dev] 1 error generated.

comment:9 Changed 8 months ago by Matthias Köppe

comment:10 Changed 8 months ago by Frédéric Chapoton

in the README

note: this file is outdated now that GMP-ECM moved to gitlab

comment:12 Changed 8 months ago by git

Commit: 39e07273d3d8c49dbec053f08d9a600f4f30b4124a00afb93113b585e60ac1bab16e65683d6ad5af

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

4a00afbbuild/pkgs/ecm/SPKG.rst: Update repo URL

comment:13 Changed 8 months ago by François Bissey

Cc: François Bissey added

comment:14 Changed 8 months ago by Dima Pasechnik

It would be good to have 7.0.5 tagged on the git repo.

comment:15 Changed 8 months ago by Dima Pasechnik

the tarball is missing https://gitlab.inria.fr/zimmerma/ecm/-/raw/master/torsions.h

comment:16 Changed 8 months ago by Paul Zimmermann

I've put a new tarball at https://members.loria.fr/PZimmermann/ecm-7.0.5a-dev.tar.gz. Is it better?

comment:17 Changed 8 months ago by git

Commit: 4a00afb93113b585e60ac1bab16e65683d6ad5afd9150d6770089df6706769079ce64ca4e3d863c7

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

d9150d6build/pkgs/ecm: Use 7.0.5a-dev

comment:18 Changed 8 months ago by Matthias Köppe

Builds and passes the test suite on macOS 12.3.1 (Intel)

comment:19 Changed 8 months ago by Matthias Köppe

Description: modified (diff)

comment:20 Changed 8 months ago by François Bissey

I am not keen to package something that is not a proper release. Any plans or roadmap to the next ecm release?

comment:21 Changed 7 months ago by Dima Pasechnik

Status: newneeds_review

on Debian stable x86_64 I get

sage -t --warn-long 111.0 --random-seed=252573809850839320636415076260300606363 src/sage/interfaces/ecm.py
**********************************************************************
File "src/sage/interfaces/ecm.py", line 717, in sage.interfaces.ecm.ECM.time
Failed example:
    ecm.time(n, 35)                  # random output
Exception raised:
    Traceback (most recent call last):
      File "/home/dimpase/work/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 695, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/dimpase/work/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 1093, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.ecm.ECM.time[1]>", line 1, in <module>
        ecm.time(n, Integer(35))                  # random output
      File "/home/dimpase/work/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/interfaces/ecm.py", line 770, in time
        while next(out_lines) != title_curves:
    StopIteration
**********************************************************************
File "src/sage/interfaces/ecm.py", line 720, in sage.interfaces.ecm.ECM.time
Failed example:
    ecm.time(n, 30, verbose=True)     # random output
Exception raised:
    Traceback (most recent call last):
      File "/home/dimpase/work/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 695, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/dimpase/work/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/doctest/forker.py", line 1093, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.interfaces.ecm.ECM.time[2]>", line 1, in <module>
        ecm.time(n, Integer(30), verbose=True)     # random output
      File "/home/dimpase/work/sage/local/var/lib/sage/venv-python3.9/lib/python3.9/site-packages/sage/interfaces/ecm.py", line 770, in time
        while next(out_lines) != title_curves:
    StopIteration
**********************************************************************
1 item had failures:
   2 of   4 in sage.interfaces.ecm.ECM.time
    [47 tests, 2 failures, 4.13 s]
----------------------------------------------------------------------
sage -t --warn-long 111.0 --random-seed=252573809850839320636415076260300606363 src/sage/interfaces/ecm.py  # 2 doctests failed

comment:22 Changed 7 months ago by Dima Pasechnik

Status: needs_reviewneeds_work

comment:23 Changed 7 months ago by Paul Zimmermann

if these failures are due to upstream, please report them: https://gitlab.inria.fr/zimmerma/ecm

comment:24 Changed 7 months ago by Dima Pasechnik

Has anything changed in ECM's time API since 7.0.4?

comment:25 in reply to:  24 Changed 7 months ago by Paul Zimmermann

Replying to dimpase:

Has anything changed in ECM's time API since 7.0.4?

which "time API" do you mean? In ecm.h there is no time function.

comment:26 in reply to:  20 Changed 7 months ago by Paul Zimmermann

Replying to fbissey:

I am not keen to package something that is not a proper release. Any plans or roadmap to the next ecm release?

I plan to release 7.0.5 soon.

comment:27 Changed 7 months ago by Dima Pasechnik

OK, what changed a bit is the output of ECM, thus we need to do

  • src/sage/interfaces/ecm.py

    a b class ECM(SageObject): 
    754754            <BLANKLINE>
    755755            Expected curves: 4911, Expected time: 32.25m
    756756        """
    757         title_curves = 'Expected number of curves to find a factor of n digits:'
     757        title_curves = 'Expected number of curves to find a factor of n digits'
    758758        title_time = 'Expected time to find a factor of n digits:'
    759759        n = self._validate(n)
    760760        B1 = self.recommended_B1(factor_digits)
    class ECM(SageObject): 
    767767            return
    768768
    769769        out_lines = iter(out.splitlines())
    770         while next(out_lines) != title_curves:
     770        while next(out_lines)[:len(title_curves)] != title_curves:
    771771            pass
    772772        header_curves = next(out_lines)
    773773        curve_count_table = next(out_lines)

to let these tests pass (it prints a bit more there now).

No issue on ECM side.

Last edited 7 months ago by Dima Pasechnik (previous) (diff)

comment:28 Changed 7 months ago by Dima Pasechnik

Authors: Matthias Koeppe
Branch: u/mkoeppe/upgrade_ecm_to_7_0_5u/dimpase/packages/ecm/vers7_0_5
Commit: d9150d6770089df6706769079ce64ca4e3d863c7f2619864f27c34b751e84d6a4d41d77cfbe39d9a
Reviewers: Dima Pasechnik
Status: needs_workneeds_review

New commits:

70c42f1build/pkgs/ecm: Update to 7.0.5-dev, drop patch
38ec9b6build/pkgs/ecm/SPKG.rst: Update repo URL
0d5f5e6build/pkgs/ecm: Use 7.0.5a-dev
f261986account for differences in output for 7.0.5 vs 7.0.4

comment:29 Changed 7 months ago by Dima Pasechnik

added the patch from comment:27, rebased over rc3

comment:30 Changed 7 months ago by Paul Zimmermann

comment:31 Changed 7 months ago by git

Commit: f2619864f27c34b751e84d6a4d41d77cfbe39d9a6a7e098ffa40ae04057404c0f7fa7635e3e45a8b

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

6a7e098released ecm 7.0.5

comment:32 Changed 7 months ago by Dima Pasechnik

Is there a way to skip these long hashes in the URL? Anyhow, this is the update, tested on macOS so far.

comment:33 Changed 7 months ago by Dima Pasechnik

Description: modified (diff)

comment:34 in reply to:  32 Changed 7 months ago by Paul Zimmermann

Replying to dimpase:

Is there a way to skip these long hashes in the URL? Anyhow, this is the update, tested on macOS so far.

I believe no. If you look at https://gitlab.inria.fr/zimmerma/ecm/-/releases/7.0.5 you first see "Source code" tarballs made automatically by gitlab, but they are of no use since they do no contain a configure file (the autotools were not run).

The "release tarball" was added manually by be, and the long hash was given to me by gitlab.

Version 0, edited 7 months ago by Paul Zimmermann (next)

comment:35 Changed 7 months ago by Dima Pasechnik

Status: needs_reviewpositive_review

lgtm

comment:36 Changed 7 months ago by Matthias Köppe

I think this should be run on GH Actions first, perhaps on top of #33316 for the platform updates

comment:37 Changed 7 months ago by Dima Pasechnik

comment:38 Changed 7 months ago by Matthias Köppe

Cc: Gonzalo Tornaría added
Status: positive_reviewneeds_review

Does it need #33861 as a dependency?

comment:39 in reply to:  38 ; Changed 7 months ago by Gonzalo Tornaría

Replying to mkoeppe:

Does it need #33861 as a dependency?

Indeed https://git.sagemath.org/sage.git/commit?id=f2619864f27c34b751e84d6a4d41d77cfbe39d9a seems to do similar as my patch, so you can just keep this ticket and ignore mine.

This shows that trac search is not very good: https://trac.sagemath.org/search?q=ecm doesn't show this ticket (no, I didn't look at the second page, given that the first page shows nothing relevant; yes, searching "upgrade ecm" gives this as the first hit).

comment:40 Changed 7 months ago by Matthias Köppe

Priority: majorcritical

Then let's mark this one here as critical.

comment:41 Changed 7 months ago by Paul Zimmermann

since #33861 was marked as duplicate, maybe we can turn this ticket back as "positive review"?

comment:42 Changed 7 months ago by Matthias Köppe

Status: needs_reviewpositive_review

comment:43 in reply to:  39 Changed 7 months ago by Samuel Lelièvre

Replying to tornaria:

This shows that trac search is not very good: https://trac.sagemath.org/search?q=ecm doesn't show this ticket

The Trac "query" engine often beats its "search" engine.

https://trac.sagemath.org/query?order=id&desc=1&summary=~ecm

I added a few custom search engines to my browser to send trac queries like the above, querying fields "summary", "description", "author", "reviewer", etc.

Then typing sagetracsummary ecm (choose your keyword) in the url bar goes to the above.

Or to query only non-closed tickets:

https://trac.sagemath.org/query?order=id&desc=1&status=!closed&summary=~ecm

comment:44 Changed 7 months ago by Volker Braun

Branch: u/dimpase/packages/ecm/vers7_0_56a7e098ffa40ae04057404c0f7fa7635e3e45a8b
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.