#6449 closed defect (fixed)
Additive abelian groups
Reported by: | davidloeffler | Owned by: | joyner |
---|---|---|---|
Priority: | major | Milestone: | sage-4.5.2 |
Component: | group theory | Keywords: | abelian group |
Cc: | jhpalmieri, mhansen | Merged in: | sage-4.5.2.alpha0 |
Authors: | David Loeffler | Reviewers: | John Cremona, Jim Stankewicz |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description
This is the results of the Abelian groups project at SD16. This is not really finished, but I am going to be pretty busy for the next few weeks so I am posting it here anyway in the hope that somebody else will chip in and finish it off.
This relies on all of the patches at #5882.
The new functionality:
- A new class for additive abelian groups. This is very similar to the finitely presented modules class, but with a slightly different print representation. This is mostly finished (although the string representation of morphisms needs a little work).
- A class for "additive abelian groups embedded in an arbitrary parent". Think rational points on an elliptic curve, for instance, which should be able to derive from this class and transparently inherit code for things like morphisms and subgroups. This is a lot less complete.
Attachments (4)
Change History (45)
comment:1 Changed 12 years ago by
- Cc jhpalmieri added
comment:2 Changed 12 years ago by
- Summary changed from Additive abelian groups to [with patches, not ready for review] Additive abelian groups
comment:3 Changed 12 years ago by
I raised this issue in http://trac.sagemath.org/sage_trac/ticket/6291 , that abelian group has different interface that other groups. In particular, they have no identity function, as opposed to all other groups I've tried in Sage. It's a minor thing, but it makes it annoying when you have code that works with arbitrary group. I've looked at the patch, and it doesn't seem to implement it. I figured I should just complain about it now instead of later :)
comment:4 Changed 11 years ago by
- Keywords abelian group added
- Report Upstream set to N/A
I'm hoping that we can sort this out and get it finished!
Comments on the patches:
- Lines 47-53 of patch 1. Not sure I understand the issue here. Isn't each coordinate just reduced modulo the appropriate integer (if positive)?
- Some functionality (e.g. annihilator()) is presumably inherited from one of the base classes. I think it would help if this was written down explicitly in comments, especially as there is more than one base class.
- As well as an is_multiplicative() returning False, should tere not be an is_additive() returning True? Or does that exist already from a base class?
- You mention a black-box discrete log would be desirable. We have that in sage/groups/generic.py -- does that do what is needed here? Or is the problem that that only deals with the cyclic case?
- Patch 2 looks quite simple, I have not looked at it in detail.
- Patch 3 is mainly changes on doctest output, but also shows how some code can be simplified using the new framework, which is good.
Now I'll actually try applying the patches to 4.3 and see how it looks.
comment:5 Changed 11 years ago by
- Reviewers set to John Cremona, Jim Stankewicz
- Status changed from needs_work to needs_review
We (Jim Stankewicz and John Cremona) have applied all three patches to sage-4.4.4. A very small amount of rebasing was needed. We have so far tested all of additive_abelian_groups, homology, and elliptic_curves; some more fixing was needed which is in the 4th patch. Some of the changes in this last patch were just fixing some mistakes made in rebasing manually.
We are now testing the whole Sage library, but the ticket is ready for review (again!)
comment:6 Changed 11 years ago by
- Summary changed from [with patches, not ready for review] Additive abelian groups to Additive abelian groups
comment:7 Changed 11 years ago by
- Milestone changed from sage-feature to sage-4.5
comment:8 Changed 11 years ago by
- Status changed from needs_review to positive_review
Positive review on John Cremona's patch, and the rest (that I looked at) looks good too.
comment:9 follow-up: ↓ 10 Changed 11 years ago by
Can you upload your rebased versions of the patches?
comment:10 in reply to: ↑ 9 Changed 11 years ago by
Replying to davidloeffler:
Can you upload your rebased versions of the patches?
I think the 4th (review) patch does what is necessary. Does it? I remember doing some manual editing after applying the first three (there were some hunks rejected), and that would be reflected in the last patch.
If that does not work, ask again. I still have that queue on my laptop.
comment:11 follow-up: ↓ 12 Changed 11 years ago by
I don't think it works like that. Perhaps the best thing is if you qfold what you've got on your laptop into a single new rebased patch.
comment:12 in reply to: ↑ 11 Changed 11 years ago by
Replying to davidloeffler:
I don't think it works like that. Perhaps the best thing is if you qfold what you've got on your laptop into a single new rebased patch.
Sorry, this will have to wait, as I am falling asleep and it has not worked so far.
comment:13 follow-up: ↓ 14 Changed 11 years ago by
- Status changed from positive_review to needs_work
Sorry about that. There was only trivial rebasing needed to the first patch (it fixed some small bugs in mrange, and those had already been fixed elsewhere, so it was possible to just ignore the failing hunks). The patch named trac_6449-1a-abgps.patch can therefore replace the one named trac_6449-1-abgps.patch, followed by -2, -3 and -review. These apply to 4.5.alpha0 (and probably also to 4.4.4, but my 4.4.4 build is currently devoted to the pari upgrade). I am currently testing the whole sage library -- done (see below).
If a single folded patch is still required I should be able to do that, but applying 4 patches is not as bad as on some tickets (and they are logically separate).
There is one doctest failure:
File "/storage/jec/sage-4.5.alpha0/devel/sage-6449/sage/groups/abelian_gps/abelian_group.py", line 538: sage: bool(T) # indirect doctest Expected: False Got: True
I suggest changing the code of that one-line function to
return self.order()>1
but I don't actually know what is going on there as it's an indirect doctest!
comment:14 in reply to: ↑ 13 Changed 11 years ago by
Replying to cremona:
Sorry about that. There was only trivial rebasing needed to the first patch (it fixed some small bugs in mrange, and those had already been fixed elsewhere, so it was possible to just ignore the failing hunks).
Yes, that part of the patch got factored out as #6561.
There is one doctest failure [...] but I don't actually know what is going on there as it's an indirect doctest!
Hmm. I see what's happening: that doctest assumes that elliptic curve torsion subgroups are an instance of the old AbelianGroup
class, while the new patch makes them an instance of my new AdditiveAbelianGroup
. Patch coming up.
David
comment:15 Changed 11 years ago by
- Status changed from needs_work to needs_review
Here's a patch. I haven't dealt with the more general problem of checking that __nonzero__
is consistently implemented -- that's a matter for another ticket, if anybody cares -- but with this patch the offending doctest does pass.
comment:16 Changed 11 years ago by
- Status changed from needs_review to positive_review
Looks good, so I am putting this back to positive review (originally positively reviewed by Robert Bradshaw).
comment:17 Changed 11 years ago by
- Status changed from positive_review to needs_work
I actually made that last patch without the other patches applied :-) So I've now realised that there is a problem with applying those patches; whatever I do it won't apply cleanly. If I apply
trac_6449-1a-abgps.patch trac_6449-2-homology.patch trac_6449-3-elliptic.patch
then I get a bunch of rejects. If I just delete the .rej files and apply trac_6449-review.patch
I get even more rejects, and a whole load of doctests fail in elliptic curves. I spent a while fixing them -- most were trivial (although tedious), but one stubborn failure in BSD.py required me to add half a dozen lines of new code to get the coercion system to behave itself; this isn't something that can be left to the release manager!
So I urge you to go through your personal patch stack, check that everything's qrefreshed, qfold the lot, check it applies 100% cleanly and upload it. (Make sure you don't qpop patches that applied with rejects without qrefreshing them first, even if you didn't make any changes yourself before creating/applying the next patch -- you need to qrefresh so hg knows you've dealt with the rejects. I just made that mistake.)
Alternatively, I can fold and upload what I've got here, and point you to the minor change I had to make to get BSD.py to pass.
David
comment:18 Changed 11 years ago by
This does not make any sense at all. I took a fresh clone of 4.5.alpha0 and applied the four patches with rejects and all relevant tests (except that little one) passed. Can you try again with a pristine build?
Since Jim and I already went through those minor doctest failures fixing them, it's a great pity that you repeated the work.
comment:19 Changed 11 years ago by
I made a completely fresh clone. Here's what happened:
masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel$ cd sage-abgp/ masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ mysage -hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6449/trac_6449-1a-abgps.patch http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6449/trac_6449-2-homology.patch http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6449/trac_6449-3-elliptic.patch http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6449/trac_6449-review.patch http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6449/trac_6449-doctest_fix.patch adding trac_6449-1a-abgps.patch to series file adding trac_6449-2-homology.patch to series file adding trac_6449-3-elliptic.patch to series file adding trac_6449-review.patch to series file adding trac_6449-doctest_fix.patch to series file masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ mysage -hg qpush applying trac_6449-1a-abgps.patch now at: trac_6449-1a-abgps.patch masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ mysage -hg qpush applying trac_6449-2-homology.patch now at: trac_6449-2-homology.patch masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ mysage -hg qpush applying trac_6449-3-elliptic.patch patching file sage/schemes/elliptic_curves/ell_finite_field.py Hunk #3 FAILED at 1205 Hunk #5 FAILED at 1295 2 out of 6 hunks FAILED -- saving rejects to file sage/schemes/elliptic_curves/ell_finite_field.py.rej patching file sage/schemes/elliptic_curves/ell_torsion.py Hunk #6 succeeded at 102 with fuzz 2 (offset 0 lines). Hunk #7 succeeded at 149 with fuzz 2 (offset 0 lines). patching file sage/schemes/elliptic_curves/padics.py Hunk #2 FAILED at 611 1 out of 2 hunks FAILED -- saving rejects to file sage/schemes/elliptic_curves/padics.py.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh trac_6449-3-elliptic.patch masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ find -name "*.rej" | xargs rm; mysage -hg qrefresh masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ mysage -hg qpush applying trac_6449-review.patch patching file sage/schemes/elliptic_curves/ell_finite_field.py Hunk #1 FAILED at 1418 Hunk #2 FAILED at 1444 2 out of 2 hunks FAILED -- saving rejects to file sage/schemes/elliptic_curves/ell_finite_field.py.rej patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh trac_6449-review.patch masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ find -name "*.rej" | xargs rm; mysage -hg qrefresh masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ mysage -hg qpushapplying trac_6449-doctest_fix.patch now at: trac_6449-doctest_fix.patch masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ find -name "*.rej" | xargs rm; mysage -hg qrefresh rm: missing operand Try `rm --help' for more information. masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$ mysage -b; mysage -tp 16 sage/schemes/elliptic_curves/ ---------------------------------------------------------- sage: Building and installing modified Sage library files. [snip] Testing that Sage starts... Yes, Sage starts. Global iterations: 1 File iterations: 1 Using cached timings to run longest doctests first. Doctesting 38 files doing 16 jobs in parallel sage -t sage/schemes/elliptic_curves/__init__.py [0.1 s] sage -t sage/schemes/elliptic_curves/ec_database.py [2.5 s] sage -t sage/schemes/elliptic_curves/all.py [2.7 s] sage -t sage/schemes/elliptic_curves/cm.py [2.8 s] sage -t sage/schemes/elliptic_curves/ell_padic_field.py [2.9 s] sage -t sage/schemes/elliptic_curves/ell_local_data.py [5.8 s] sage -t sage/schemes/elliptic_curves/ell_wp.py [3.9 s] sage -t sage/schemes/elliptic_curves/ell_field.py [8.8 s] sage -t sage/schemes/elliptic_curves/descent_two_isogeny.pyx [9.1 s] sage -t sage/schemes/elliptic_curves/ell_tate_curve.py [9.3 s] sage -t sage/schemes/elliptic_curves/gp_simon.py [3.8 s] sage -t sage/schemes/elliptic_curves/ell_torsion.py [10.5 s] sage -t sage/schemes/elliptic_curves/gp_cremona.py [4.9 s] sage -t sage/schemes/elliptic_curves/gal_reps.py [7.3 s] sage -t sage/schemes/elliptic_curves/kodaira_symbol.py [2.4 s] sage -t sage/schemes/elliptic_curves/mod5family.py [2.9 s] sage -t sage/schemes/elliptic_curves/modular_parametrization.py [3.6 s] sage -t sage/schemes/elliptic_curves/monsky_washnitzer.py [4.0 s] sage -t sage/schemes/elliptic_curves/padic_height.py [3.2 s] sage -t sage/schemes/elliptic_curves/BSD.py Exception raised by doctesting framework. Use -verbose for details. [20.7 s] sage -t sage/schemes/elliptic_curves/lseries_ell.py [9.0 s] sage -t sage/schemes/elliptic_curves/sea.py [2.1 s] sage -t sage/schemes/elliptic_curves/weierstrass_morphism.py [3.6 s] sage -t sage/schemes/elliptic_curves/constructor.py [29.8 s] sage -t sage/schemes/elliptic_curves/ell_finite_field.py ********************************************************************** File "/storage/masiao/sage-4.5.alpha1/devel/sage-abgp/sage/schemes/elliptic_curves/ell_finite_field.py", line 1421: sage: E.abelian_group() Expected: (Multiplicative Abelian Group isomorphic to C10, ... Got: Additive abelian group isomorphic to Z/10 embedded in Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 2*x + 5 over Finite Field of size 11 ********************************************************************** File "/storage/masiao/sage-4.5.alpha1/devel/sage-abgp/sage/schemes/elliptic_curves/ell_finite_field.py", line 1427: sage: E.abelian_group() Expected: (Multiplicative Abelian Group isomorphic to C22 x C2, ... Got: Additive abelian group isomorphic to Z/2 + Z/22 embedded in Abelian group of points on Elliptic Curve defined by y^2 = x^3 + 2*x + 5 over Finite Field of size 41 ********************************************************************** File "/storage/masiao/sage-4.5.alpha1/devel/sage-abgp/sage/schemes/elliptic_curves/ell_finite_field.py", line 1434: sage: E.abelian_group() Expected: (Multiplicative Abelian Group isomorphic to C26 x C26, ... Got: Additive abelian group isomorphic to Z/26 + Z/26 embedded in Abelian group of points on Elliptic Curve defined by y^2 = x^3 + (a^4+a^3+2*a^2+2*a)*x + (2*a^5+2*a^3+2*a^2+1) over Finite Field in a of size 3^6 ********************************************************************** File "/storage/masiao/sage-4.5.alpha1/devel/sage-abgp/sage/schemes/elliptic_curves/ell_finite_field.py", line 1441: sage: E.abelian_group() Expected: (Multiplicative Abelian Group isomorphic to C1031352, ... Got: Additive abelian group isomorphic to Z/1031352 embedded in Abelian group of points on Elliptic Curve defined by y^2 = x^3 + (2*a^2+48*a+27)*x + (89*a^2+76*a+24) over Finite Field in a of size 101^3 ********************************************************************** File "/storage/masiao/sage-4.5.alpha1/devel/sage-abgp/sage/schemes/elliptic_curves/ell_finite_field.py", line 1447: sage: E.abelian_group() Expected: (Trivial Abelian Group, ()) Got: Trivial group embedded in Abelian group of points on Elliptic Curve defined by y^2 + y = x^3 + x + 1 over Finite Field of size 2 ********************************************************************** 1 items had failures: 5 of 24 in __main__.example_24 ***Test Failed*** 5 failures. For whitespace errors, see the file /home/masiao/.sage//tmp/.doctest_ell_finite_field.py [34.9 s] sage -t sage/schemes/elliptic_curves/period_lattice.py [18.6 s] sage -t sage/schemes/elliptic_curves/formal_group.py [33.2 s] sage -t sage/schemes/elliptic_curves/ell_modular_symbols.py [39.3 s] sage -t sage/schemes/elliptic_curves/padics.py [21.6 s] sage -t sage/schemes/elliptic_curves/ell_egros.py [41.1 s] sage -t sage/schemes/elliptic_curves/ell_curve_isogeny.py [45.4 s] sage -t sage/schemes/elliptic_curves/ell_generic.py [48.5 s] sage -t sage/schemes/elliptic_curves/ell_point.py [51.0 s] sage -t sage/schemes/elliptic_curves/padic_lseries.py [42.0 s] sage -t sage/schemes/elliptic_curves/ell_rational_field.py [62.7 s] sage -t sage/schemes/elliptic_curves/ell_number_field.py [67.5 s] sage -t sage/schemes/elliptic_curves/sha_tate.py [55.0 s] sage -t sage/schemes/elliptic_curves/heegner.py [97.1 s] ---------------------------------------------------------------------- The following tests failed: sage -t devel/sage-abgp/sage/schemes/elliptic_curves/BSD.py # Exception from doctest framework sage -t devel/sage-abgp/sage/schemes/elliptic_curves/ell_finite_field.py # 5 doctests failed ---------------------------------------------------------------------- Timings have been updated. Total time for all tests: 109.3 seconds masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgp$
The only explanation I can come up with is that this has something to do with #9127, which was more or less the only non-graph-theory patch merged in 4.5.alpha1.
David
comment:20 Changed 11 years ago by
I can't be #9127 -- all that does is add three dots to a doctest output!
I'll try again with alpha1, but I am seeing tutees all afternoon.
comment:21 Changed 11 years ago by
- Status changed from needs_work to needs_review
I guess the moral here is that one can't rely on an old outdated patch always failing in exactly the same way, i.e. the selection of exactly which hunks apply and which don't seems to be a bit random.
Anyway: another issue here is the danger of conflicts with other positively-reviewed patches which touch elliptic curves. Right now there are 13 of these from 11 tickets: 7930, 8680, 9087, 9110, 9266, 9287, 9302, 9313, 9324, 9342 and 9372.
So I downloaded all of these, applied the patches from this ticket (including the review patch), and cleaned up the mess, and everything was fine except for one thing: there wasn't a coercion from the torsion subgroup of an elliptic curve to the full M-W group, and more generally from an additive abelian group wrapper to its "universe", which causes the doctest failure in BSD.py reported in my previous message. I added an UnwrappingMorphism? class to additive_abelian_wrapper which does exactly that. With that change, all doctests seem to pass.
It turns out that the patch thus obtained actually doesn't conflict with any of the other positively reviewed patches above except #9278, but you need that one installed. It passes doctests both with and without the other 10 tickets. If someone could quickly check that the new UnwrappingMorphism? looks plausible, then I think this will finally be ready to merge.
comment:22 Changed 11 years ago by
I'm very grateful to the time you have spent on this. I guess it is not surprising that there are lots of e.c. patches in the pipeline given that we are nearing the end of a 2-week Sage Days on Computing with Elliptic Curves.
I'm about to look through those patches, which I am re-listing here: #7930, #8680, #9087, #9110, #9266, #9287, #9302, #9313, #9324, #9342 and #9372 so that trac can make links to make the job easier!
comment:23 Changed 11 years ago by
Well: on my newly built 4.5.alpha1 and in a new clone:
jec@selmer%pwd /home/jec/storage/sage-4.5.alpha1 jec@selmer%cd devel/sage-6449/ jec@selmer%hg qinit jec@selmer%hg qimp ~/trac_6449-1a-abgps.patch adding trac_6449-1a-abgps.patch to series file jec@selmer%hg qpush applying trac_6449-1a-abgps.patch now at: trac_6449-1a-abgps.patch jec@selmer%hg qimp ~/trac_6449-2-homology.patch adding trac_6449-2-homology.patch to series file jec@selmer%hg qpush applying trac_6449-2-homology.patch now at: trac_6449-2-homology.patch jec@selmer%hg qimp ~/trac_6449-3-elliptic.patch adding trac_6449-3-elliptic.patch to series file jec@selmer%hg qpush applying trac_6449-3-elliptic.patch now at: trac_6449-3-elliptic.patch jec@selmer%hg qimp ~/trac_6449-review.patch adding trac_6449-review.patch to series file jec@selmer%hg qpush applying trac_6449-review.patch now at: trac_6449-review.patch jec@selmer%sage -b
which I am now testing. Then I'll try again with the new folded patch!
comment:24 Changed 11 years ago by
The alpha1 test showed up these:
sage -t devel/sage-6449/sage/groups/abelian_gps/abelian_group.py # 1 doctests failed sage -t devel/sage-6449/sage/schemes/elliptic_curves/BSD.py # Exception from doctest framework sage -t devel/sage-6449/sage/tests/book_stein_ent.py # 4 doctests failed sage -t devel/sage-6449/sage/modular/modsym/space.py # 3 doctests failed
where the first one is the little one fixed by youur doctest patch and the others are trivialities caused by the different output of torsion groups now.
Next I'll test the new folded patch... applies ok, starting to test.
comment:25 Changed 11 years ago by
The folded patch should deal with the first two. I also saw the other two you mention, which are both coming from the fact that invariants
now returns a tuple instead of a string; and if you test the docs as well I imagine you will see a failure in constructions/elliptic_curves.rst
. These remaining issues should be fixed by the patch I am about to upload.
comment:26 Changed 11 years ago by
Strangely, when I applied the combined (folded) patch to 4.5.alpha1 (which applied cleanly) and then ran sage -t on everything there were thousands or errors including many totally unrelated. I don't think that is a trustworthy test and will try again tomorrow.
comment:27 Changed 11 years ago by
OK, did sage -ba and then a complete test (this is with the big folded patch but not the ones-that-got-away) and found only these:
sage -t devel/sage-6449/sage/all.py # 0 doctests failed sage -t devel/sage-6449/sage/tests/book_stein_ent.py # 4 doctests failed sage -t devel/sage-6449/sage/modular/modsym/space.py # 3 doctests failed
of which the first can be ignored, the second and third are treated by ones-that-got-away.
But applying the ones-that-got-away fails as follows:
applying trac_6449_ones_that_got_away.patch patching file sage/modular/modsym/space.py Hunk #1 FAILED at 2203 Hunk #2 FAILED at 2213 Hunk #3 FAILED at 2226 3 out of 3 hunks FAILED -- saving rejects to file sage/modular/modsym/space.py.rej
which is odd. Still, we are nearly there.
comment:28 Changed 11 years ago by
comment:29 Changed 11 years ago by
OK, I will try that -- in which case we might ignore trac_6449_ones_that_got_away.1.patch which I just uploaded.
comment:30 Changed 11 years ago by
Please list exactly which patches one needs to apply, in order, to 4.5.alpha1, including those on the other ticket. It's not clear from the above whether the tabs patches need applying before all these or in between or what: I found no order in which everything works, except when I ignore the tabs patches and apply just
trac_6449_everything.patch trac_6449_ones_that_got_away.1.patch
comment:31 follow-up: ↓ 32 Changed 11 years ago by
I'm mystified that it's not working for you. I just created yet another new clone on selmer, downloading the patches afresh from trac, and it works fine there too. See session transcript below.
One order of patches that works (for me) is:
trac_9287.coverage_for_elliptic_curves_part1.patch trac_9287.coverage_for_elliptic_curves_part2.patch trac_6449_everything.patch trac_8680-untabify-4.5.alpha1.patch trac_8680-tinyfix.patch trac_6449_ones_that_got_away.patch
#8680 can also come earlier in the sequence -- everything except trac_6449_ones_that_got_away.patch is independent of it.
david@rockhopper:~> ssh selmer.warwick.ac.uk [...] Last login: Fri Jul 2 09:42:07 2010 from 92.17.192.129 masiao@selmer:~$ cd /storage/masiao/sage-4.5.alpha1/ masiao@selmer:/storage/masiao/sage-4.5.alpha1$ ./sage -b main [...] masiao@selmer:/storage/masiao/sage-4.5.alpha1$ ./sage -clone abgps [...] masiao@selmer:/storage/masiao/sage-4.5.alpha1$ cd devel/sage-abgps masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgps$ ../../sage -hg qimport http://trac.sagemath.org/sage_trac/raw-attachment/ticket/9287/trac_9287.coverage_for_elliptic_curves_part1.patch \ http://trac.sagemath.org/sage_trac/raw-attachment/ticket/9287/trac_9287.coverage_for_elliptic_curves_part2.patch \ http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6449/trac_6449_everything.patch \ http://trac.sagemath.org/sage_trac/raw-attachment/ticket/8680/trac_8680-untabify-4.5.alpha1.patch \ http://trac.sagemath.org/sage_trac/raw-attachment/ticket/8680/trac_8680-tinyfix.patch \ http://trac.sagemath.org/sage_trac/raw-attachment/ticket/6449/trac_6449_ones_that_got_away.patch adding trac_9287.coverage_for_elliptic_curves_part1.patch to series file adding trac_9287.coverage_for_elliptic_curves_part2.patch to series file adding trac_6449_everything.patch to series file adding trac_8680-untabify-4.5.alpha1.patch to series file adding trac_8680-tinyfix.patch to series file adding trac_6449_ones_that_got_away.patch to series file masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgps$ ../../sage -hg qpush -a applying trac_9287.coverage_for_elliptic_curves_part1.patch applying trac_9287.coverage_for_elliptic_curves_part2.patch applying trac_6449_everything.patch applying trac_8680-untabify-4.5.alpha1.patch applying trac_8680-tinyfix.patch applying trac_6449_ones_that_got_away.patch now at: trac_6449_ones_that_got_away.patch masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgps$ export MAKE="make -j16"; ../../sage -b [builds with no errors] masiao@selmer:/storage/masiao/sage-4.5.alpha1/devel/sage-abgps$
comment:32 in reply to: ↑ 31 Changed 11 years ago by
Replying to davidloeffler:
I'm mystified that it's not working for you. I just created yet another new clone on selmer, downloading the patches afresh from trac, and it works fine there too. See session transcript below.
One order of patches that works (for me) is:
trac_9287.coverage_for_elliptic_curves_part1.patch trac_9287.coverage_for_elliptic_curves_part2.patch trac_6449_everything.patch trac_8680-untabify-4.5.alpha1.patch trac_8680-tinyfix.patch trac_6449_ones_that_got_away.patch
For what it's worth, this worked for me on a fresh install of 4.5.alpha1 on two different machines. All tests pass on sage.math.
comment:33 Changed 11 years ago by
OK, so I rebuilt my 4.5.alpha1 from scratch and now the patches do apply ok in the order you give. I'll test too but do not expect any surprises.
However, I definitely think it is unacceptable (to the release manager, for a start) for the patches on one ticket to have to be applied with two patches from another ticket *in between* them! That will surely cause a delay.
comment:34 Changed 11 years ago by
#8680 has now been merged, and in one of my tests, I applied both of the patches from #8680 before both of the patches here, and it worked fine. So I think at this point just merging the patches from #9287 before the patches here should be good enough. No need to merge patches from any ticket in between two of the patches from another one.
John (Cremona): I'll leave this to you to restore to "positive review" if your tests pass.
comment:35 Changed 11 years ago by
- Status changed from needs_review to positive_review
My tests passed , so let's roll.
comment:36 follow-up: ↓ 37 Changed 11 years ago by
- Cc mhansen added
What about all of the functions in additive_abelian_wrapper.py with no doctests?
comment:37 in reply to: ↑ 36 Changed 11 years ago by
Replying to mhansen:
What about all of the functions in additive_abelian_wrapper.py with no doctests?
Good point. David, any chance you could oblige?
Changed 11 years ago by
comment:38 Changed 11 years ago by
Here's a patch with the missing doctests. The current situation is that one needs to apply:
- the #9287 patches
- trac_6449_everything.patch
- trac_6449_ones_that_got_away.patch
- trac_6449_more_doctests.patch
If someone with trac admin rights could delete all the other patches that would be helpful.
David
comment:39 follow-up: ↓ 41 Changed 11 years ago by
I checked that the patches as specified by David, and repeated here:
- trac_9287.coverage_for_elliptic_curves_part1.patch
- trac_9287.coverage_for_elliptic_curves_part2.patch
- trac_6449_everything.patch
- trac_6449_ones_that_got_away.patch
- trac_6449_more_doctests.patch
apply ok to 4.5.alpha4. The new doctests look fine and pass, and coverage in this directory is now 100%.
I have therefore left the tag as "positive review" and hope that mhansen is now happy!
comment:40 Changed 11 years ago by
- Merged in set to sage-4.5.2.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
comment:41 in reply to: ↑ 39 Changed 11 years ago by
Replying to cremona:
I checked that the patches as specified by David, and repeated here:
- trac_9287.coverage_for_elliptic_curves_part1.patch
- trac_9287.coverage_for_elliptic_curves_part2.patch
- trac_6449_everything.patch
- trac_6449_ones_that_got_away.patch
- trac_6449_more_doctests.patch
apply ok to 4.5.alpha4. The new doctests look fine and pass, and coverage in this directory is now 100%.
I have therefore left the tag as "positive review" and hope that mhansen is now happy!
For the record, I did not apply trac_6449_ones_that_got_away.1.patch to 4.5.2.alpha0.
Here are three patches: the first is the new classes, and the second make alterations to the two places where abelian groups are used "in an additive way" in the Sage library to convert them over (which I did chiefly in order to get a feel for how well it would owrk in practice).