#10335 closed enhancement (fixed)
Add domains for permutation groups
Reported by: | Mike Hansen | Owned by: | joyner |
---|---|---|---|
Priority: | major | Milestone: | sage-4.7.2 |
Component: | group theory | Keywords: | sd31 |
Cc: | Sage Combinat CC user, Jason B. Hill, Rob Beezer, Jeroen Demeyer | Merged in: | sage-4.7.2.alpha3 |
Authors: | Mike Hansen, Jason Hill, David Loeffler | Reviewers: | Robert Miller, Rob Beezer, Nicolas Borie, Nicolas M. Thiéry |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | #10334 | Stopgaps: |
Description (last modified by )
Often, one wants to define a permutation group on a set other than {1,2,...,n}
such as say {'a', 'b', 'c', 'd'}
.
For example,
sage: G = PermutationGroup([ [('c','d')], [('a','c')] ]) sage: G.orbit('a') ['a', 'c', 'd']
Apply:
Attachments (7)
Change History (87)
comment:1 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Status: | new → needs_review |
comment:2 Changed 12 years ago by
Cc: | Sage Combinat CC user Jason B. Hill added |
---|
comment:3 Changed 12 years ago by
Reviewers: | → Robert Miller |
---|---|
Work issues: | → needs rebase |
Applying to sage-4.6.2.alpha0
on top of #10334, I get:
applying trac_10335-permgroup_domain-mh.patch patching file sage/groups/perm_gps/permgroup.py Hunk #5 FAILED at 321 1 out of 59 hunks FAILED -- saving rejects to file sage/groups/perm_gps/permgroup.py.rej patching file sage/groups/perm_gps/permgroup_named.py Hunk #10 succeeded at 313 with fuzz 2 (offset 0 lines). Hunk #11 succeeded at 315 with fuzz 1 (offset -49 lines). patch failed, unable to continue (try -v) patch failed, rejects left in working dir errors during apply, please fix and refresh trac_10335-permgroup_domain-mh.patch
The failure:
$ cat sage/groups/perm_gps/permgroup.py.rej --- permgroup.py +++ permgroup.py @@ -320,7 +322,7 @@ sage: G = PermutationGroup([[(1,2,3),(4,5)],[(3,4)]]) sage: TestSuite(G).run() """ - def __init__(self, gens=None, gap_group=None, canonicalize=True, category = None): + def __init__(self, gens=None, gap_group=None, canonicalize=True, category=None, domain=None): r""" INPUT:
Looks trivial to fix, and I'm excited to see this feature in Sage. Thank you, Mike!
comment:4 Changed 12 years ago by
Description: | modified (diff) |
---|
comment:5 Changed 12 years ago by
Work issues: | needs rebase |
---|
This was most likely a minor conflict with Rob's permgroup patches which were merged late in the 4.6.2 release cycle.
Only apply: trac_10335-permgroup_domain-mh.2.patch
comment:6 Changed 12 years ago by
Status: | needs_review → needs_work |
---|
Ah, the test fail, and that's because the patch still did not commute perfectly with trac_8359-coxeter-groups-permutation-nt.patch. we need to have this patch add the category argument to init of permutation groups, and remove that part from 8359.
Mike: let me know if you can do it shortly, and then #8359 is fair game. Otherwise I'll handle it.
comment:7 Changed 12 years ago by
Cc: | Rob Beezer added |
---|
comment:8 Changed 12 years ago by
Description: | modified (diff) |
---|
I've fixed the issue along with updating the patch at #8359.
Only apply: trac_10335-permgroup_domain-mh.patch
comment:9 Changed 12 years ago by
This looks real good, Mike. I spent some time with this today. I know its not done yet, but hopefully the following will be helpful.
I applied the patch from sage-combinat for #8359 to 4.7.alpha1, as best I could tell (used patch from http://combinat.sagemath.org/hgwebdir.cgi/patches/rev/508d4942a6d2). One hunk failure at sage/combinat/root_system/all.py. Then #10334 had a hunk failure in sage/groups/perm_gps/permgroup.py and #10335 had a had failure in sage/categories/pushout.py. All easy to fix by hand, and maybe a result of not getting the right thing from the sage-combinat server.
Passed all tests in sage/groups/perm_gps, except two, so not too far off. Both doctest failures look very minor, and a result of routines I wrote that have been added recently.
********************************************************************** File "/sage/dev/devel/sage-main/sage/groups/perm_gps/permgroup.py", line 1920: sage: G.subgroups() Expected: [Permutation Group with generators [()], Permutation Group with generators [(2,3)], Permutation Group with generators [(1,2)], Permutation Group with generators [(1,3)], Permutation Group with generators [(1,2,3)], Permutation Group with generators [(1,3,2), (1,2)]] Got: [Permutation Group with generators [()], Permutation Group with generators [(2,3)], Permutation Group with generators [(1,2)], Permutation Group with generators [(1,3)], Permutation Group with generators [(1,2,3)], Permutation Group with generators [(1,2), (1,3,2)]] ********************************************************************** File "/sage/dev/devel/sage-main/sage/groups/perm_gps/permgroup.py", line 2082: sage: A.cosets(S) Expected: Traceback (most recent call last): ... ValueError: Subgroup of SymmetricGroup(3) generated by [(1,2)] is not a subgroup of AlternatingGroup(3) Got: Traceback (most recent call last): File "/sage/dev/local/bin/ncadoctest.py", line 1231, in run_one_test self.run_one_example(test, example, filename, compileflags) File "/sage/dev/local/bin/sagedoctest.py", line 38, in run_one_example OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags) File "/sage/dev/local/bin/ncadoctest.py", line 1172, in run_one_example compileflags, 1) in test.globs File "<doctest __main__.example_61[29]>", line 1, in <module> A.cosets(S)###line 2082: sage: A.cosets(S) File "/sage/dev/local/lib/python/site-packages/sage/groups/perm_gps/permgroup.py", line 2099, in cosets raise ValueError("%s is not a subgroup of %s" % (S, self)) ValueError: Subgroup of (Symmetric group of order 3! as a permutation group) generated by [(1,2)] is not a subgroup of Alternating group of order 3!/2 as a permutation group **********************************************************************
I think the stabilizer() routine needs some work.
try: postition = self._domain_to_gap[point] except KeyError: return self.subgroup(gens=self.gens()) return self.subgroup(gap_group=gap.Stabilizer(self, point))
"position" is mis-spelled as first defined, and in the return, I think "point" should be replaced with "position". With these changes, this seems to work properly with a non-trivial domain in play.
Not a comprehensive look, but hopefully of use. Let me know what I can do to help (including assisting with documentation). It'd be nice if there was a patch, or something, at #8359 explaining how best to apply that work.
Rob
comment:10 follow-up: 11 Changed 12 years ago by
Hi Mike,
For the record: I rebased trac_10335-permgroup_domain-mh.patch on the sage-combinat server on top of #9949
comment:11 Changed 12 years ago by
Hi Mike!
Replying to nthiery:
For the record: I rebased trac_10335-permgroup_domain-mh.patch on the sage-combinat server on top of #9949
With Rob we are both at Sage days 30, and would love to push permutation groups in. Rob is going to rebase #10334, and I'll finalize the review. #10335 itself needs rebase on 3.4.7, and has quite a few failing doctests, though most look harmless (it's just that the output of permutation groups have changed). Would there be any chance for you to work on this early this week?
Thanks!
Cheers,
Nicolas
comment:13 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
I've fixed the doctest failures and updated the patch.
comment:14 Changed 12 years ago by
Status: | needs_review → needs_work |
---|
I ran make ptestlong
and got quite a few failures. I even tried it twice - not sure what is up.
- Many were in sage/doc
- Many were simply the new version of
_repr_
for the symmetric group, and its subgroups - The
latex()
method is putting a comma between the cycles within each generator. Is this desired?return ",".join(["(" + ",".join([latex(x) for x in cycle])+")" for cycle in self.cycle_tuples()])
At a minimum it is causing doctest failures. - 73 failures in sage/rings/number_field/galois_group.py, likely all the same root cause
Saw these in the patch itself:
- a stray "deprecated_function_alias" in a docstring
``selfs``
in a docstring
comment:15 Changed 12 years ago by
Dependencies: | → #10334 |
---|
comment:16 Changed 12 years ago by
Status: | needs_work → needs_review |
---|
I'm pretty sure that this patches addresses all of the above problems.
Apply trac_10335-permgroup_domain-mh.patch
comment:18 Changed 12 years ago by
I was very close to set a positive review for this patch but I got this faillure :
nicolas@lancelot:/opt/sage/devel/sage-combinat$ sage -t sage/groups/perm_gps/permgroup.py sage -t "devel/sage-combinat/sage/groups/perm_gps/permgroup.py" ********************************************************************** File "/opt/sage/devel/sage-combinat/sage/groups/perm_gps/permgroup.py", line 2550: sage: G.isomorphism_to(H) Expected: Permutation group morphism: From: Permutation Group with generators [(2,3), (1,2,3)] To: Permutation Group with generators [(1,2,4), (1,4)] Defn: [(2,3), (1,2,3)] -> [(2,4), (1,2,4)] Got: Permutation group morphism: From: Permutation Group with generators [(2,3), (1,2,3)] To: Permutation Group with generators [(1,2,4), (1,4)] Defn: [(2,3), (1,2,3)] -> [(2,4), (1,4,2)] ********************************************************************** 1 items had failures: 1 of 9 in __main__.example_73
Is this a random test ? Is this test really unrandonized (sorry for such a word) ? I am running a 11.4 Ubuntu up to date on a macbook santa rosa 4,1. I got this faillure both using the sage combinat queue and a separate branch with only 10334 and 10335.
Also, in sage/groups/perm-gps/permgroup.py line 1119, there is still the micro typo : postition --> position
Anyway, It is a very very nice work and improvement of permutation groups.
comment:19 Changed 12 years ago by
The failure you see is at #11427 . I'll put up a new patch fixing the typo.
Changed 12 years ago by
Attachment: | trac_10335-permgroup_domain-mh.patch added |
---|
comment:20 Changed 12 years ago by
Reviewers: | Robert Miller → Robert Miller, Rob Beezer, Nicolas Borie |
---|---|
Status: | needs_review → positive_review |
Nice shoot! I confirm that I use a lot the Gap database especially for Transitive Groups. The optional package was installed on my machine.
Thanks for your work and your rapidity for fixing this last typo.
I give this patch a positive review.
comment:21 Changed 12 years ago by
Description: | modified (diff) |
---|---|
Milestone: | sage-5.0 → sage-4.7.1 |
comment:22 follow-up: 23 Changed 11 years ago by
Description: | modified (diff) |
---|
For the record: when applying this patch with the sage-combinat queue on sage-4.7.2, I got an import loop, and used the extra patch to fix it.
comment:23 follow-up: 24 Changed 11 years ago by
Replying to nthiery:
For the record: when applying this patch with the sage-combinat queue on sage-4.7.2, I got an import loop, and used the extra patch to fix it.
On 4.7.2?
comment:24 Changed 11 years ago by
comment:25 Changed 11 years ago by
Status: | positive_review → needs_work |
---|
comment:26 follow-up: 27 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
New patch needs review.
comment:27 Changed 11 years ago by
Keywords: | sd31 added |
---|---|
Status: | needs_review → needs_work |
Replying to jdemeyer:
New patch needs review.
Almost passes all tests on 4.7.1.alpha2 and I get no import problems.
But the German documentation has old _repr_
behavior. Aargh.
sage -t -long -force_lib ooks/devel/sage/doc/de/tutorial/tour_groups.rst # 1 doctests failed sage -t -long -force_lib ooks/devel/sage/doc/de/tutorial/interfaces.rst # 1 doctests failed
Patch coming up, maybe.
Changed 11 years ago by
Attachment: | trac_10335_german_tutorial_doc.patch added |
---|
comment:28 follow-up: 33 Changed 11 years ago by
Status: | needs_work → needs_review |
---|
German doc patch should solve above problems. Gotta run - ticket needs "apply" block....
comment:29 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:30 follow-up: 32 Changed 11 years ago by
Note that trac_10335-permgroup_domain-import_loop_fix-nt.patch should have its commit message changed.
comment:31 Changed 11 years ago by
Milestone: | sage-4.7.1 → sage-4.7.2 |
---|
Changed 11 years ago by
Attachment: | trac_10335-permgroup_domain-import_loop_fix-nt.patch added |
---|
Merge only if needed
comment:32 Changed 11 years ago by
Replying to jdemeyer:
Note that trac_10335-permgroup_domain-import_loop_fix-nt.patch should have its commit message changed.
Oops, done.
comment:33 Changed 11 years ago by
Status: | needs_review → positive_review |
---|
Replying to rbeezer:
German doc patch should solve above problems. Gotta run - ticket needs "apply" block....
Sounds good to me assuming the patch bot is happy.
comment:34 follow-up: 35 Changed 11 years ago by
Status: | positive_review → needs_work |
---|
The Russian tutorial (#9378) also needs to be fixed:
sage -t -force_lib devel/sage/doc/ru/tutorial/interfaces.rst ********************************************************************** File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.2.alpha0/devel/sage-main/doc/ru/tutorial/interfaces.rst", line 167: sage: G.center() Expected: Permutation Group with generators [()] Got: Subgroup of (Permutation Group with generators [(3,4), (1,2,3)(4,5)]) generated by [()] **********************************************************************
sage -t -force_lib devel/sage/doc/ru/tutorial/tour_groups.rst ********************************************************************** File "/mnt/usb1/scratch/jdemeyer/merger/sage-4.7.2.alpha0/devel/sage-main/doc/ru/tutorial/tour_groups.rst", line 24: sage: G.center() Expected: Permutation Group with generators [()] Got: Subgroup of (Permutation Group with generators [(3,4), (1,2,3)(4,5)]) generated by [()] **********************************************************************
Changed 11 years ago by
Attachment: | trac_10335-tutorial-languages-fixes.patch added |
---|
German and Russian language fixes combined
comment:35 follow-up: 36 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
Replying to jdemeyer:
The Russian tutorial (#9378) also needs to be fixed:
Applied #9378 and the two main patches here. (I do not understand what "merge if needed" is suppose to mean - so I included that). Rolled the Russian fixes up with the German fixes into one patch.
Passes all tests in doc directory.
Could we please review and merge this before anymore new languages surface? ;-) ;-)
Rob
comment:36 Changed 11 years ago by
Reviewers: | Robert Miller, Rob Beezer, Nicolas Borie → Robert Miller, Rob Beezer, Nicolas Borie, Nicolas M. Thiéry |
---|---|
Status: | needs_review → positive_review |
Replying to rbeezer:
Applied #9378 and the two main patches here. (I do not understand what "merge if needed" is suppose to mean - so I included that).
See comment 23 above.
Rolled the Russian fixes up with the German fixes into one patch.
Passes all tests in doc directory.
Could we please review and merge this before anymore new languages surface? ;-) ;-)
I checked your new patch, and am happy with it. Back to positive review. Thanks!
comment:37 Changed 11 years ago by
Merged in: | → sage-4.7.2.alpha0 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:38 Changed 11 years ago by
Merged in: | sage-4.7.2.alpha0 |
---|---|
Resolution: | fixed |
Status: | closed → new |
Work issues: | → Solaris doctest error |
Unfortunately, there is a doctest error on Solaris (to be more precise, the buildbot machine mark2):
sage -t -long -force_lib devel/sage/sage/groups/perm_gps/permgroup.py ********************************************************************** File "/home/buildbot/build/sage/mark-1/mark_full/build/sage-4.7.2.alpha0/devel/sage-main/sage/groups/perm_gps/permgroup.py", line 811: sage: G.gens_small() Expected: [('a','b'), ('a','b','c')] Got: [('b','c'), ('a','b','c')] **********************************************************************
comment:39 Changed 11 years ago by
What's the best way to test a fix on that machine? Do I have to log into mark2 and do a full build or is there a way to access that build?
comment:41 Changed 11 years ago by
jdemeyer:
What's the best way to test a fix on that machine? Do I have to log into mark2 and do a full build or is there a way to access that build?
Changed 11 years ago by
Attachment: | trac_10335-solaris_issue.patch added |
---|
apply over previous patches
comment:42 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Status: | new → needs_review |
I don't have access to a Solaris machine to test this on; but, looking at the code, it's clear that (a) this Sage function just wraps the corresponding one in Gap, (b) the answer returned on Solaris is just as much a valid answer as that returned on other systems, and (c) there is already a perfectly good doctest for that function.
I strongly suggest that we change this doctest to something less sensitive, given that there is a huge pile of other code waiting on this. I have just uploaded a patch which does this.
comment:43 Changed 11 years ago by
Description: | modified (diff) |
---|
This has now bitrotted: several doctests fail in trivial ways on sage 4.7.2.alpha2. I've just uploaded a patch that fixes these.
comment:44 Changed 11 years ago by
Work issues: | Solaris doctest error |
---|
I've tested this on the Sun machine "mark" on skynet, and all doctests pass (except for a pre-existing error in matrix_double_dense
which has nothing to do with this patch. Can someone please visually check that the two most recent patches trac_10335-solaris_issue.patch and trac_10335-updates.patch are morally sound? I'd be really disappointed if this bitrotted yet again.
comment:45 Changed 11 years ago by
FWIW, the "pre-existing error in matrix_double_dense
" is a known issue coming from #10795, which has apparently been de-merged from the next release for this reason.
comment:46 Changed 11 years ago by
Authors: | Mike Hansen → Mike Hansen, David Loeffler |
---|---|
Status: | needs_review → positive_review |
David's two recent patches look good to me, and update from a point where this had a positive review.
Applies, builds, passes long tests on 4.7.2.alpha2 and 64-bit Ubuntu 11.04. So, back to positive review with a hope it merges it OK, finally.
comment:47 follow-up: 48 Changed 11 years ago by
I'm not sure whether removing a doctest is the proper way to fix a doctest error... ;-)
comment:48 Changed 11 years ago by
Replying to leif:
I'm not sure whether removing a doctest is the proper way to fix a doctest error... ;-)
I agree. But in this case, I agree with David's argument. This patch implements a highly desirable feature with many doctests. And if I understand right, GAP uses some probabilistic algorithms, so we could tie ourselves in knots trying to rewrite this doctest.
My understanding (and some grep'ing appears to confirm) is that this is a new test. Would the patch have gone in without it if it had never appeared? I think so. If an existing test were failing, I'd be a lot less cavalier about dropping it.
Anyway, my $0.02 worth.
comment:49 follow-up: 50 Changed 11 years ago by
I think the correct fix would to be to add one of the current_randstate().set_seed_gap()
lines to the beginning of that function.
comment:50 Changed 11 years ago by
Replying to mhansen:
I think the correct fix would to be to add one of the
current_randstate().set_seed_gap()
lines to the beginning of that function.
Or otherwise at least tag the doctest with # random
, # not tested
, # platform-specific
(?) or whatever, since doing so documents the different behaviour.
comment:51 Changed 11 years ago by
I agree that in an ideal world it would be better to have another doctest for this function, but as Rob points out, there was no such doctest before this ticket anyway. So it seems absurd to hold up all of the other new functionality added by this ticket, and the two other positively-reviewed tickets depending on it, for such a reason.
If it matters that much, perhaps we should have a new ticket "improve doctests for wrapper of Gap function xxx".
comment:52 Changed 11 years ago by
I agree with leif that adding a simple "# random" is an easy solution which at least documents the problem.
comment:53 follow-ups: 54 56 Changed 11 years ago by
comment:54 follow-up: 55 Changed 11 years ago by
Replying to davidloeffler:
Aaargh! Do what you like, just *MERGE IT SOMEHOW*! If we miss the boat with this one again, and consequently #11422, #11598, #5048, #11601 and #10546 end up still trapped in purgatory, I will have to think very seriously about whether working on Sage in future is a productive use of my time.
Well, to be honest, it's not my problem that this ticket bit-rottened. I started merging tickets "officially" into alpha3 about two weeks ago IIRC (including some you participated in); this ticket got positive review 20 hours ago.
I agree that it's a pity, not just because other already positively reviewed tickets -- three of the other ones you mentioned still need review -- depend on this one, but as I told Rob yesterday, it would have to get rebased anyway, because of (at least) #11749, and I'm not going to back the latter out (or try to reapply it on top of tickets that haven't had positive review recently, again having to manually resolve conflicts and errors, some showing up only later during testing).
We haven't yet talked about the release plans for the final Sage 4.7.2, but I think there'll still be an alpha4 before we go into feature freeze, as there are other important things that still have to get fixed and should be solved in a final.
So IMHO it would be best to wait for the alpha3, hopefully released within the next days, and then rebase this ticket (and if necessary the others) on that.
[I didn't touch the tickets having milestone "sage-pending" (including #11422 and #11598) at all, since I don't know why Jeroen put them there, although I have to admit I also didn't try to find out, mainly because of lack of time. I early did ping the authors and reviewers of all other tickets that were holding up other positively reviewed ones though, with some success.]
W.r.t. the doctest with varying output:
It would IMHO be best to give the alternative, also correct result as an example, too, marking both # not tested
, with an appropriate comment.
(And perhaps add a real doctest which tests a property common to both.)
comment:55 Changed 11 years ago by
comment:56 follow-up: 59 Changed 11 years ago by
Replying to davidloeffler:
Aaargh! Do what you like, just *MERGE IT SOMEHOW*!
Leif - thanks for the help with this one. With classes starting, I really wasn't in a position to be chasing the evolving alpha, so I had to pass on that offer, but appreciate the willingness.
David - How about the following?
- Wait for alpha3 (which sounds big already).
- I mark the disputed test
# random
.
- I rebase on alpha3 in preparation for alpha4.
- You review (2) and (3).
- We quickly identify if the tickets in Permutation Group Purgatory needing changes/rebasing.
Lemme know and I'll carve out time to make it happen.
Rob
comment:57 follow-up: 58 Changed 11 years ago by
That sounds like a good solution -- thanks for volunteering to do (2) and (3). I can take care of (5).
My apologies, particularly to Leif, for my outburst earlier.
comment:58 Changed 11 years ago by
Status: | positive_review → needs_work |
---|
Replying to davidloeffler:
That sounds like a good solution -- thanks for volunteering to do (2) and (3). I can take care of (5).
Great! We have a plan. Thanks for taking (5).
Leif and/or Jeroen - feel free to ping us if we need to move quickly on something.
At the ready, Rob
comment:59 follow-up: 60 Changed 11 years ago by
Replying to rbeezer:
- Wait for alpha3 (which sounds big already).
[...]
- I rebase on alpha3 in preparation for alpha4.
[...]
- We quickly identify if the tickets in Permutation Group Purgatory needing changes / rebasing.
For the impatient, an updated tarball and source tree for upgrading can be found in
http://sage.math.washington.edu/home/leif/Sage/release/sage-4.7.2.alpha3-prerelease/.
I don't expect further changes to the alpha3 release, except for a few only fixing some doctest errors due to numerical noise below
sage/rings/polynomial/
andsage/matrix/
,
and perhaps some clean-up like adding ticket numbers to commit messages lacking them, so it should be safe to (re)base patches on this version.
comment:60 Changed 11 years ago by
Changed 11 years ago by
Attachment: | trac_10335-permgroup_domain-rebase.patch added |
---|
patch against 4.7.2.alpha3 prerelease
comment:61 follow-up: 63 Changed 11 years ago by
Description: | modified (diff) |
---|---|
Status: | needs_work → needs_review |
I had a few minutes free this afternoon, so I've done a rebased patch myself. This is just the existing patches rebased and folded, but with the doctest in permgroup.py
marked as random rather than deleted, and with examples of the output I got on a Linux and a Solaris machine.
Rob: any chance you could review this? Apologies if you'd already started rebasing this yourself as per our earlier agreement -- in that case feel free to ignore my patch and I'll review yours.
Leif: you might be interested to hear that this prerelease builds and passes long doctests without problems on my 64-bit Linux machine -- no numerical noise or anything like that.
comment:62 follow-up: 64 Changed 11 years ago by
David - no problem, I'll do a review. I have no free minutes today. So tonight. ;-)
Probably about 8-10 hours from now. Thanks for hopping on this one.
comment:63 Changed 11 years ago by
Replying to davidloeffler:
Leif: you might be interested to hear that this prerelease builds and passes long doctests without problems on my 64-bit Linux machine -- no numerical noise or anything like that.
If you also tell me what CPU you built on... (e.g. top of /proc/cpuinfo
, i.e., only for the first core...)
Might be interesting.
comment:64 Changed 11 years ago by
Replying to rbeezer:
Probably about 8-10 hours from now.
My build was interrupted (long story) and now two files are giving segfaults when I doctest, with or without the patch. Don't panic - likely entirely all my fault. So anyway, I'm starting over and building overnight. Stay tuned.
Rob
comment:65 follow-up: 66 Changed 11 years ago by
alpha3-prerelease segfaults during testing, without patch. Errors, followed by system info. I can follow up with more testing, if requested.
The following tests failed: sage -t -long -force_lib devel/sage/sage/modular/modsym/ambient.py # 0 doctests failed sage -t -long -force_lib devel/sage/sage/modular/hecke/ambient_module.py # 0 doctests failed sage -t -long -force_lib devel/sage/sage/modular/hecke/hecke_operator.py # 0 doctests failed ---------------------------------------------------------------------- Total time for all tests: 2701.6 seconds make: *** [ptestlong] Error 128
rob@lava:/sage/sage-4.7.2.alpha3-prerelease$ ./sage -t -long -force_lib devel/sage/sage/modular/modsym/ambient.py sage -t -long -force_lib "devel/sage/sage/modular/modsym/ambient.py" /sage/sage-4.7.2.alpha3-prerelease/local/lib/libcsage.so(print_backtrace+0x31)[0x7fd8e335834f] /sage/sage-4.7.2.alpha3-prerelease/local/lib/libcsage.so(sigdie+0x14)[0x7fd8e3358381] /sage/sage-4.7.2.alpha3-prerelease/local/lib/libcsage.so(sage_signal_handler+0x20e)[0x7fd8e3357fb4] /lib/x86_64-linux-gnu/libpthread.so.0(+0xfc60)[0x7fd8e484fc60] /sage/sage-4.7.2.alpha3-prerelease/local/lib/libatlas.so(ATL_dJIK44x44x44TN44x44x0_a1_b0+0x9d)[0x7fd8da12c3ed] ------------------------------------------------------------------------ Unhandled SIGSEGV: A segmentation fault occurred in Sage. This probably occurred because a *compiled* component of Sage has a bug in it and is not properly wrapped with sig_on(), sig_off(). You might want to run Sage under gdb with 'sage -gdb' to debug this. Sage will now terminate. ------------------------------------------------------------------------ Segmentation fault [5.9 s] ---------------------------------------------------------------------- The following tests failed: sage -t -long -force_lib "devel/sage/sage/modular/modsym/ambient.py" Total time for all tests: 5.9 seconds
rob@lava:/sage/sage-4.7.2.alpha3-prerelease$ uname -a Linux lava 2.6.38-10-generic #46-Ubuntu SMP Tue Jun 28 15:07:17 UTC 2011 x86_64 x86_64 x86_64 GNU/Linux
rob@lava:/sage/sage-4.7.2.alpha3-prerelease$ gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/lib/x86_64-linux-gnu/gcc/x86_64-linux-gnu/4.5.2/lto-wrapper Target: x86_64-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro 4.5.2-8ubuntu4' --with-bugurl=file:///usr/share/doc/gcc-4.5/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.5 --enable-shared --enable-multiarch --with-multiarch-defaults=x86_64-linux-gnu --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib/x86_64-linux-gnu --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.5 --libdir=/usr/lib/x86_64-linux-gnu --enable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-plugin --enable-gold --enable-ld=default --with-plugin-ld=ld.gold --enable-objc-gc --disable-werror --with-arch-32=i686 --with-tune=generic --enable-checking=release --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu Thread model: posix gcc version 4.5.2 (Ubuntu/Linaro 4.5.2-8ubuntu4)
rob@lava:/sage/sage-4.7.2.alpha3-prerelease$ cat /proc/cpuinfo processor : 0 vendor_id : GenuineIntel cpu family : 6 model : 42 model name : Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz stepping : 7 cpu MHz : 1600.000 cache size : 8192 KB physical id : 0 siblings : 8 core id : 0 cpu cores : 4 apicid : 0 initial apicid : 0 fpu : yes fpu_exception : yes cpuid level : 13 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts rep_good nopl xtopology nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm sse4_1 sse4_2 x2apic popcnt aes xsave avx lahf_lm ida arat epb xsaveopt pln pts dts tpr_shadow vnmi flexpriority ept vpid bogomips : 6822.05 clflush size : 64 cache_alignment : 64 address sizes : 36 bits physical, 48 bits virtual power management:
comment:66 follow-up: 67 Changed 11 years ago by
Replying to rbeezer:
alpha3-prerelease segfaults during testing, without patch. Errors, followed by system info. I can follow up with more testing, if requested.
That's either due to Linaro, or, more likely, an ATLAS problem, since our version afaik doesn't yet support Sandy Bridge (second generation "Core i").
You could try using your system's ATLAS (may have to install it with your package manager first), by doing:
$ export SAGE_ATLAS_LIB=/usr/lib64/atlas # the directory; usual place on 64-bit Linux $ rm $SAGE_ROOT/spkg/installed/atlas-* $ cd $SAGE_ROOT $ env SAGE_UPGRADING=yes make # or "... make build"
The SAGE_UPGRADING=yes
is important to (really) rebuild all dependent packages as well.
comment:67 Changed 11 years ago by
Replying to leif:
$ export SAGE_ATLAS_LIB=/usr/lib64/atlas # the directory; usual place on 64-bit Linux ...
Might be /usr/lib/x86_64-linux-gnu/atlas/
on your system; you should check this before attempting to build... ;-)
comment:68 follow-up: 69 Changed 11 years ago by
Thanks, Leif. Ubuntu installs ATLAS into /usr/lib64/atlas-base
. Then setting up everything as you describe, suitably adjusted yields:
Unable to find one of liblapack, libcblas, libatlas or libf77blas in the directory /usr/lib64/atlas-base Set SAGE_ATLAS_LIB to the directory containing liblapack, libcblas, libatlas and libf77blas (either .a or .so extensions) if you wish to use existing ATLAS libraries. For more details, see
I've installed the packages liblapack3gf, libblas3gf
but cannot see anything to provide libf77blas and do not know if I am getting libcblas. Furthermore, these all go to different directories.
Any advice? My Sandy Bridge motherboard is about 8 months old. Will anybody with a newer chip have to go through a drill like this? Would Volker's ATLAS ticket be a better/workable approach?
comment:69 Changed 11 years ago by
Replying to rbeezer:
I've installed the packages
liblapack3gf, libblas3gf
but cannot see anything to provide libf77blas and do not know if I am getting libcblas. Furthermore, these all go to different directories.
I've asked Volker on the IRC for help. (Perhaps drop in if you can.)
If some libraries are simply in different directories, you could just create symbolic links to them in the .../atlas-base/
directory. (Although there IMHO should^{TM} be a directory containing all; are there some more self-contained ATLAS packages you could install?)
My Sandy Bridge motherboard is about 8 months old.
That shouldn't matter; Intel's bug just causes a SATA part of the chipset to die earlier.
Whether AVX is available depends on your board (and chipset); if you have on-board graphics, AVX isn't usable.
Will anybody with a newer chip have to go through a drill like this?
Simon King did successfully build the prerelease on a (Sandy Bridge) Core i3 with Debian x86_64; there only five doctests (in three files) failed due to numerical noise (the same doctests that failed on some other "rare" and older platforms).
comment:70 follow-up: 73 Changed 11 years ago by
The following bug in Ubuntu launchpad says that the symlinks for the standard libraries are not created: https://bugs.launchpad.net/ubuntu/+source/atlas/+bug/769180 So you either create them manually (in a directory of your choice), or you compile ATLAS again. For the latter, you should probably try to set
export SAGE_ATLAS_ARCH=Corei7,SSE3,SSE2,SSE1
This works only with the new atlas spkg (4.7.2.alpha3+)
comment:71 follow-up: 72 Changed 11 years ago by
AVX is just the CPU instruction set extension du jour. The version of ATLAS that we are shipping knows nothing about AVX. All 2011 sandy bridge cpus have it, irrespective of on-board graphics.
comment:72 Changed 11 years ago by
Replying to vbraun:
AVX is just the CPU instruction set extension du jour. The version of ATLAS that we are shipping knows nothing about AVX.
Aha. Wasn't sure about that. But does 3.8.4 "know" Sandy Brigde already?
All 2011 sandy bridge cpus have it, irrespective of on-board graphics.
They have it, but you can only use it when the CPU doesn't also act as a GPU. And afaik that's hard-wired, i.e., you even cannot use it if your motherboard has on-board graphics you don't actually use.
comment:73 Changed 11 years ago by
Replying to vbraun:
This works only with the new atlas spkg (4.7.2.alpha3+)
I guess you meant alpha2+.
Apparently alpha2 in contrast did not segfault for Rob (on that machine), despite having the same ATLAS spkg. At least that's my understanding.
comment:74 Changed 11 years ago by
Oh yes, it was merged in alpha2.
This is now completely OT, but my (Sandy Bridge) Thinkpad W520 uses the on-board GPU and has AVX:
[vbraun@volker-laptop-two ~]$ cat /proc/cpuinfo | head -24 processor : 0 vendor_id : GenuineIntel cpu family : 6 model : 42 model name : Intel(R) Core(TM) i7-2720QM CPU @ 2.20GHz stepping : 7 cpu MHz : 2201.000 cache size : 6144 KB physical id : 0 siblings : 8 core id : 0 cpu cores : 4 apicid : 0 initial apicid : 0 fpu : yes fpu_exception : yes cpuid level : 13 wp : yes flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx rdtscp lm constant_tsc arch_perfmon pebs bts nopl xtopology nonstop_tsc aperfmperf pni pclmulqdq dtes64 monitor ds_cpl vmx smx est tm2 ssse3 cx16 xtpr pdcm sse4_1 sse4_2 x2apic popcnt aes xsave avx lahf_lm ida arat epb xsaveopt pln pts dts tpr_shadow vnmi flexpriority ept vpid bogomips : 4385.92 clflush size : 64 cache_alignment : 64 address sizes : 36 bits physical, 48 bits virtual power management:
comment:75 follow-up: 76 Changed 11 years ago by
Leif and Volker,
Thanks for the quick help, and I apologize to the group theorists for diverting this ticket. I've got to attend to some other things the rest of today, but I'll see if I can get the symlinks in place with guidance from that bug report.
I have Sandy Bridge with no on-board graphics. I have built and tested alpha2 - this is my first problem like this.
Rob
comment:76 Changed 11 years ago by
Replying to rbeezer:
Leif and Volker,
Thanks for the quick help, and I apologize to the group theorists for diverting this ticket.
Yep, we can continue the discussion on e.g. this sage-release thread, or -- preferably -- a new one there.
comment:77 Changed 11 years ago by
Authors: | Mike Hansen, David Loeffler → Mike Hansen, Jason Hill, David Loeffler |
---|---|
Status: | needs_review → positive_review |
alpha3 prerelease build is fixed and passes long tests, with and without this rebased patch. To quote William "merge that sucker"! In other words, positive review.
Second line of patch summary message credits Jason Hill - I'll add him to the authors, and Mike's name is still on the main patch.
David - feel free to cc me on any of the follow-on tickets that might need review, especially if they are cosmetic changes. I have not yet looked at them at all.
Leif - thanks for your attention to this one and help with my build challenges.
Everybody else - my apologies again for getting this ticket off the tracks. I need to post a summary on sage-release.
Rob
comment:78 Changed 11 years ago by
comment:79 Changed 11 years ago by
Merged in: | → sage-4.7.2.alpha3 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:80 Changed 11 years ago by
Hello and sorry to add a comment on a closed ticket. I have a patch which depend on it and the patchbot try to apply 2 patches instead one here.
apply trac_10335-permgroup_domain-rebase.patch
This depends