Opened 22 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:  sage9.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: 
Description (last modified by )
(from #30600)
official 7.0.5 tarball: see checksums.ini
Change History (44)
comment:1 Changed 22 months ago by
comment:2 Changed 21 months ago by
Milestone:  sage9.3 → sage9.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
Milestone:  sage9.4 → sage9.5 

comment:4 Changed 15 months ago by
Milestone:  sage9.5 → sagepending 

comment:5 Changed 7 months ago by
Description:  modified (diff) 

comment:6 Changed 7 months ago by
Branch:  → u/mkoeppe/upgrade_ecm_to_7_0_5 

comment:7 Changed 7 months ago by
Commit:  → 39e07273d3d8c49dbec053f08d9a600f4f30b412 

Milestone:  sagepending → sage9.7 
New commits:
39e0727  build/pkgs/ecm: Update to 7.0.5dev, drop patch

comment:8 Changed 7 months ago by
[ecm7.0.5dev] gcc DHAVE_CONFIG_H I. DOUTSIDE_LIBECM g march=native g O2 fPIC c o ecmmain.o `test f 'main.c'  echo './'`main.c [ecm7.0.5dev] main.c:55:10: fatal error: 'torsions.h' file not found [ecm7.0.5dev] #include "torsions.h" /* to benefit from more torsion groups */ [ecm7.0.5dev] ^~~~~~~~~~~~ [ecm7.0.5dev] 1 error generated.
comment:10 Changed 7 months ago by
in the README
note: this file is outdated now that GMPECM moved to gitlab
comment:12 Changed 7 months ago by
Commit:  39e07273d3d8c49dbec053f08d9a600f4f30b412 → 4a00afb93113b585e60ac1bab16e65683d6ad5af 

Branch pushed to git repo; I updated commit sha1. New commits:
4a00afb  build/pkgs/ecm/SPKG.rst: Update repo URL

comment:13 Changed 7 months ago by
Cc:  François Bissey added 

comment:15 Changed 7 months ago by
the tarball is missing https://gitlab.inria.fr/zimmerma/ecm//raw/master/torsions.h
comment:16 Changed 7 months ago by
I've put a new tarball at https://members.loria.fr/PZimmermann/ecm7.0.5adev.tar.gz. Is it better?
comment:17 Changed 7 months ago by
Commit:  4a00afb93113b585e60ac1bab16e65683d6ad5af → d9150d6770089df6706769079ce64ca4e3d863c7 

Branch pushed to git repo; I updated commit sha1. New commits:
d9150d6  build/pkgs/ecm: Use 7.0.5adev

comment:19 Changed 7 months ago by
Description:  modified (diff) 

comment:20 followup: 26 Changed 7 months ago by
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
Status:  new → needs_review 

on Debian stable x86_64 I get
sage t warnlong 111.0 randomseed=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/venvpython3.9/lib/python3.9/sitepackages/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/venvpython3.9/lib/python3.9/sitepackages/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/venvpython3.9/lib/python3.9/sitepackages/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/venvpython3.9/lib/python3.9/sitepackages/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/venvpython3.9/lib/python3.9/sitepackages/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/venvpython3.9/lib/python3.9/sitepackages/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 warnlong 111.0 randomseed=252573809850839320636415076260300606363 src/sage/interfaces/ecm.py # 2 doctests failed
comment:22 Changed 7 months ago by
Status:  needs_review → needs_work 

comment:23 Changed 7 months ago by
if these failures are due to upstream, please report them: https://gitlab.inria.fr/zimmerma/ecm
comment:24 followup: 25 Changed 7 months ago by
Has anything changed in ECM's time API since 7.0.4?
comment:25 Changed 7 months ago by
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 Changed 7 months ago by
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
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): 754 754 <BLANKLINE> 755 755 Expected curves: 4911, Expected time: 32.25m 756 756 """ 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' 758 758 title_time = 'Expected time to find a factor of n digits:' 759 759 n = self._validate(n) 760 760 B1 = self.recommended_B1(factor_digits) … … class ECM(SageObject): 767 767 return 768 768 769 769 out_lines = iter(out.splitlines()) 770 while next(out_lines) != title_curves:770 while next(out_lines)[:len(title_curves)] != title_curves: 771 771 pass 772 772 header_curves = next(out_lines) 773 773 curve_count_table = next(out_lines)
to let these tests pass.
No issue on ECM side.
comment:28 Changed 7 months ago by
Authors:  → Matthias Koeppe 

Branch:  u/mkoeppe/upgrade_ecm_to_7_0_5 → u/dimpase/packages/ecm/vers7_0_5 
Commit:  d9150d6770089df6706769079ce64ca4e3d863c7 → f2619864f27c34b751e84d6a4d41d77cfbe39d9a 
Reviewers:  → Dima Pasechnik 
Status:  needs_work → needs_review 
comment:30 Changed 7 months ago by
I've made a proper release (https://gitlab.inria.fr/zimmerma/ecm/uploads/89f6f0d65d3e980cef33dc922004e4b2/ecm7.0.5.tar.gz) with tag 7.0.5 in the git repo
comment:31 Changed 7 months ago by
Commit:  f2619864f27c34b751e84d6a4d41d77cfbe39d9a → 6a7e098ffa40ae04057404c0f7fa7635e3e45a8b 

Branch pushed to git repo; I updated commit sha1. New commits:
6a7e098  released ecm 7.0.5

comment:32 followup: 34 Changed 7 months ago by
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
Description:  modified (diff) 

comment:34 Changed 7 months ago by
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 me, and the long hash was given to me by gitlab.
comment:36 Changed 7 months ago by
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
tests running on https://github.com/dimpase/sage/actions/runs/2314328253 (on top on #32937 and #33316)
comment:38 followup: 39 Changed 7 months ago by
Cc:  Gonzalo Tornaría added 

Status:  positive_review → needs_review 
Does it need #33861 as a dependency?
comment:39 followup: 43 Changed 7 months ago by
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
Priority:  major → critical 

Then let's mark this one here as critical.
comment:41 Changed 7 months ago by
since #33861 was marked as duplicate, maybe we can turn this ticket back as "positive review"?
comment:42 Changed 7 months ago by
Status:  needs_review → positive_review 

comment:43 Changed 7 months ago by
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 nonclosed tickets:
https://trac.sagemath.org/query?order=id&desc=1&status=!closed&summary=~ecm
comment:44 Changed 7 months ago by
Branch:  u/dimpase/packages/ecm/vers7_0_5 → 6a7e098ffa40ae04057404c0f7fa7635e3e45a8b 

Resolution:  → fixed 
Status:  positive_review → closed 
warning, 7.0.5 is not yet released! And the release number might change...