Opened 9 years ago

Closed 9 years ago

#13391 closed enhancement (fixed)

WeylCharacterRing improvement

Reported by: bump Owned by: bump
Priority: major Milestone: sage-5.4
Component: combinatorics Keywords: WeylCharacterRing, Lie
Cc: sage-combinat, bump Merged in: sage-5.4.beta1
Authors: Daniel Bump Reviewers: Anne Schilling
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by bump)

The multiplication algorithm for WeylCharacterRings is changed by this patch to an algorithm based on the Brauer-Klimyk formula. This algorithm is asymmetric, so that a*b and b*a are equal but computed differently. They are sometimes interchanged in an attempt to give the most efficient evaluation. This patch gives a modest speedup for sage -t --long weyl_characters.py but in certain cases it is a dramatic improvement. As an example, create the spin character of Spin(9):

sage: B4=WeylCharacterRing("B4",style="coroots")
sage: spin=B4(0,0,0,1)

Now try raising this to different powers before and after the patch.

Apply: trac_13391-weyl_characters.patch

Attachments (1)

trac_13391-weyl_characters.patch (22.1 KB) - added by bump 9 years ago.
Weyl Character Ring speedup of product_on_basis

Download all attachments as: .zip

Change History (42)

comment:1 Changed 9 years ago by bump

  • Description modified (diff)

comment:2 Changed 9 years ago by nthiery

Hi Dan!

For future record, can you add your timing, before and after the patch, to the ticket description?

Thanks!

comment:3 Changed 9 years ago by aschilling

My timings before the patch::

   sage: %timeit spin^3
   5 loops, best of 3: 539 ms per loop
   sage: %timeit spin^4
   5 loops, best of 3: 6.31 s per loop

and with this patch::

   sage: %timeit spin^3
   5 loops, best of 3: 25.3 ms per loop
   sage: %timeit spin^4
   5 loops, best of 3: 126 ms per loop
   sage: %timeit spin^5
   5 loops, best of 3: 842 ms per loop
   sage: %timeit spin^6
   5 loops, best of 3: 520 ms per loop

This is definitely a huge improvement!

comment:4 Changed 9 years ago by aschilling

Hi Dan,

This looks good and I am happy to give it a positive review. Here are just some minor comments about the documentation (I think after the INPUT variables there is supposed to be a double dash. Also for the html output it is nicer to use \rho than rho for example)::

    Auxiliary function for product_on_basis. 
	         
    INPUT: 
      
    - ``a'' -- a weight.

    Returns a pair `[\epsilon,b]` where `b` is a dominant weight and
    `\epsilon` = 0,1 or -1. To describe `b`, let `w` be an element of 
    the Weyl group such that `w(a+\rho)` is dominant. If 
    `w(a+\rho)-\rho` is dominant, then `\epsilon` is the sign of
    `w` and `b` is `w(a+\rho)-\rho`. Otherwise, `\epsilon` is zero. 

comment:5 follow-ups: Changed 9 years ago by bump

The improvement in timing is minor for many tasks, and sage -t --long weyl_characters.py only comes in a few seconds faster. But for tensor product decompositions where the first factor is smaller than the other, the improvement is big, as Anne's posted timings show. One can get up to the 12th or 13th tensor power of the spin representation of spin=B4(0,0,0,1), which was previously hopeless.

I took Anne's doc comment into account and reposted the patch.

Last edited 9 years ago by bump (previous) (diff)

comment:6 in reply to: ↑ 5 Changed 9 years ago by aschilling

Looks good!

Anne

comment:7 Changed 9 years ago by aschilling

  • Reviewers set to Anne Schilling
  • Status changed from new to needs_review

comment:8 Changed 9 years ago by aschilling

  • Status changed from needs_review to positive_review

comment:9 in reply to: ↑ 5 ; follow-up: Changed 9 years ago by nthiery

Replying to bump:

But for tensor product decompositions where the first factor is smaller than the other,

Would it make sense, in a later ticket, to commute the two factors if the second is smaller than the first? (I haven't looked at the code, maybe that's already done).

In any cases, nice improvement, thanks!

Cheers,

comment:10 in reply to: ↑ 9 Changed 9 years ago by nthiery

Replying to nthiery:

Would it make sense, in a later ticket, to commute the two factors if the second is smaller than the first? (I haven't looked at the code, maybe that's already done).

Also: did you try computing the powers naively (that is as a*(a*(a*(a))) rather than using the default binary powering(a22) as implemented by power? If it's better to have one small factor, it might be one of those monoids where binary powering is not the right thing to do.

Cheers,

Nicolas

comment:11 Changed 9 years ago by bump

I was wondering about these exact points. There would be some minor overhead involved in deciding whether to switch a and b before computing a*b. I think in many cases it would be worthwhile.

I was also wondering about the power, and there ... watch this!

sage: B4=WeylCharacterRing("B4",style="coroots"); spin=B4(0,0,0,1)
sage: time spin^8
1001*B4(0,0,0,0) + 3003*B4(1,0,0,0) + 5460*B4(0,1,0,0) + 7644*B4(0,0,1,0) + 8918*B4(0,0,0,2) + 3388*B4(2,0,0,0) + 8085*B4(1,1,0,0) + 12474*B4(1,0,1,0) + 15092*B4(1,0,0,2) + 5775*B4(0,2,0,0) + 11550*B4(0,1,1,0) + 15092*B4(0,1,0,2) + 6300*B4(0,0,2,0) + 10290*B4(0,0,1,2) + 4116*B4(0,0,0,4) + 1716*B4(3,0,0,0) + 4550*B4(2,1,0,0) + 7371*B4(2,0,1,0) + 9100*B4(2,0,0,2) + 4290*B4(1,2,0,0) + 9009*B4(1,1,1,0) + 12012*B4(1,1,0,2) + 5460*B4(1,0,2,0) + 9100*B4(1,0,1,2) + 3822*B4(1,0,0,4) + 1540*B4(0,3,0,0) + 3564*B4(0,2,1,0) + 4928*B4(0,2,0,2) + 2835*B4(0,1,2,0) + 4900*B4(0,1,1,2) + 2268*B4(0,1,0,4) + 825*B4(0,0,3,0) + 1540*B4(0,0,2,2) + 924*B4(0,0,1,4) + 210*B4(0,0,0,6) + 330*B4(4,0,0,0) + 924*B4(3,1,0,0) + 1540*B4(3,0,1,0) + 1925*B4(3,0,0,2) + 972*B4(2,2,0,0) + 2100*B4(2,1,1,0) + 2835*B4(2,1,0,2) + 1344*B4(2,0,2,0) + 2268*B4(2,0,1,2) + 980*B4(2,0,0,4) + 462*B4(1,3,0,0) + 1100*B4(1,2,1,0) + 1540*B4(1,2,0,2) + 924*B4(1,1,2,0) + 1617*B4(1,1,1,2) + 770*B4(1,1,0,4) + 300*B4(1,0,3,0) + 567*B4(1,0,2,2) + 350*B4(1,0,1,4) + 84*B4(1,0,0,6) + 84*B4(0,4,0,0) + 210*B4(0,3,1,0) + 300*B4(0,3,0,2) + 196*B4(0,2,2,0) + 350*B4(0,2,1,2) + 175*B4(0,2,0,4) + 84*B4(0,1,3,0) + 162*B4(0,1,2,2) + 105*B4(0,1,1,4) + 28*B4(0,1,0,6) + 14*B4(0,0,4,0) + 28*B4(0,0,3,2) + 20*B4(0,0,2,4) + 7*B4(0,0,1,6) + B4(0,0,0,8)
Time: CPU 20.33 s, Wall: 20.34 s

[Exit and restart sage to avoid caching effects ...]

sage: B4=WeylCharacterRing("B4",style="coroots"); spin=B4(0,0,0,1)
sage: time spin*(spin*(spin*(spin*(spin*(spin*(spin*spin))))))
1001*B4(0,0,0,0) + 3003*B4(1,0,0,0) + 5460*B4(0,1,0,0) + 7644*B4(0,0,1,0) + 8918*B4(0,0,0,2) + 3388*B4(2,0,0,0) + 8085*B4(1,1,0,0) + 12474*B4(1,0,1,0) + 15092*B4(1,0,0,2) + 5775*B4(0,2,0,0) + 11550*B4(0,1,1,0) + 15092*B4(0,1,0,2) + 6300*B4(0,0,2,0) + 10290*B4(0,0,1,2) + 4116*B4(0,0,0,4) + 1716*B4(3,0,0,0) + 4550*B4(2,1,0,0) + 7371*B4(2,0,1,0) + 9100*B4(2,0,0,2) + 4290*B4(1,2,0,0) + 9009*B4(1,1,1,0) + 12012*B4(1,1,0,2) + 5460*B4(1,0,2,0) + 9100*B4(1,0,1,2) + 3822*B4(1,0,0,4) + 1540*B4(0,3,0,0) + 3564*B4(0,2,1,0) + 4928*B4(0,2,0,2) + 2835*B4(0,1,2,0) + 4900*B4(0,1,1,2) + 2268*B4(0,1,0,4) + 825*B4(0,0,3,0) + 1540*B4(0,0,2,2) + 924*B4(0,0,1,4) + 210*B4(0,0,0,6) + 330*B4(4,0,0,0) + 924*B4(3,1,0,0) + 1540*B4(3,0,1,0) + 1925*B4(3,0,0,2) + 972*B4(2,2,0,0) + 2100*B4(2,1,1,0) + 2835*B4(2,1,0,2) + 1344*B4(2,0,2,0) + 2268*B4(2,0,1,2) + 980*B4(2,0,0,4) + 462*B4(1,3,0,0) + 1100*B4(1,2,1,0) + 1540*B4(1,2,0,2) + 924*B4(1,1,2,0) + 1617*B4(1,1,1,2) + 770*B4(1,1,0,4) + 300*B4(1,0,3,0) + 567*B4(1,0,2,2) + 350*B4(1,0,1,4) + 84*B4(1,0,0,6) + 84*B4(0,4,0,0) + 210*B4(0,3,1,0) + 300*B4(0,3,0,2) + 196*B4(0,2,2,0) + 350*B4(0,2,1,2) + 175*B4(0,2,0,4) + 84*B4(0,1,3,0) + 162*B4(0,1,2,2) + 105*B4(0,1,1,4) + 28*B4(0,1,0,6) + 14*B4(0,0,4,0) + 28*B4(0,0,3,2) + 20*B4(0,0,2,4) + 7*B4(0,0,1,6) + B4(0,0,0,8)
Time: CPU 0.33 s, Wall: 0.34 s

Conclusion: don't binary!

comment:12 Changed 9 years ago by bump

  • Owner changed from sage-combinat to bump

comment:13 follow-up: Changed 9 years ago by bump

I revised __pow__ and reposted the patch so it needs review again.

The revision to __pow__ appears to be another very substantial speedup. Previously, it could not compute the 16th power of the spin representation of B4 in a reasonable amount of time, and now it can -- in 3.45 seconds.

I don't see how to change the status to needs_review. I'm only given options needs_info, needs_work ... none of which are correct.

Last edited 9 years ago by bump (previous) (diff)

comment:14 in reply to: ↑ 13 Changed 9 years ago by aschilling

I can confirm that this is a huge speedup:

sage: B4=WeylCharacterRing("B4",style="coroots"); spin=B4(0,0,0,1)
sage: time spin^(16)
1844536720*B4(0,0,0,0) + 8300415240*B4(1,0,0,0) + 19920996576*B4(0,1,0,0) + 33201660960*B4(0,0,1,0) + 42140569680*B4(0,0,0,2) + 16709332640*B4(2,0,0,0) + 52634397816*B4(1,1,0,0) + 96675424560*B4(1,0,1,0) + 127247994720*B4(1,0,0,2) + 56393997660*B4(0,2,0,0) + 134271423000*B4(0,1,1,0) + 190871992080*B4(0,1,0,2) + 96675424560*B4(0,0,2,0) + 171784792872*B4(0,0,1,2) + 81802282320*B4(0,0,0,4) + 19747393120*B4(3,0,0,0) + 69115875920*B4(2,1,0,0) + 133294903560*B4(2,0,1,0) + 179028564000*B4(2,0,0,2) + 97749595944*B4(1,2,0,0) + 244373989860*B4(1,1,1,0) + 354476556720*B4(1,1,0,2) + 195499191888*B4(1,0,2,0) + 354476556720*B4(1,0,1,2) + 177238278360*B4(1,0,0,4) + 62610852304*B4(0,3,0,0) + 172499286960*B4(0,2,1,0) + 259486106880*B4(0,2,0,2) + 181124251308*B4(0,1,2,0) + 340575515280*B4(0,1,1,2) + 187664059440*B4(0,1,0,4) + 79062173190*B4(0,0,3,0) + 160557028632*B4(0,0,2,2) + 114683591880*B4(0,0,1,4) + 34405077564*B4(0,0,0,6) + 14990430000*B4(4,0,0,0) + 55404629280*B4(3,1,0,0) + 109929820000*B4(3,0,1,0) + 149492475000*B4(3,0,0,2) + 87424187760*B4(2,2,0,0) + 224856450000*B4(2,1,1,0) + 330242467500*B4(2,1,0,2) + 189958728960*B4(2,0,2,0) + 348736045680*B4(2,0,1,2) + 179390970000*B4(2,0,0,4) + 74144430360*B4(1,3,0,0) + 210159950000*B4(1,2,1,0) + 320089770000*B4(1,2,0,2) + 233025352560*B4(1,1,2,0) + 443644421220*B4(1,1,1,2) + 251499105000*B4(1,1,0,4) + 113486373000*B4(1,0,3,0) + 233345442330*B4(1,0,2,2) + 171476662500*B4(1,0,1,4) + 54323806680*B4(1,0,0,6) + 31455212880*B4(0,4,0,0) + 93616705000*B4(0,3,1,0) + 145495350000*B4(0,3,0,2) + 115335780560*B4(0,2,2,0) + 224062839000*B4(0,2,1,2) + 133370737500*B4(0,2,0,4) + 74144430360*B4(0,1,3,0) + 155563628220*B4(0,1,2,2) + 120033663750*B4(0,1,1,4) + 42251849640*B4(0,1,0,6) + 22049487460*B4(0,0,4,0) + 47975807880*B4(0,0,3,2) + 40795755000*B4(0,0,2,4) + 18847638810*B4(0,0,1,6) + 4038779745*B4(0,0,0,8) + 7475227760*B4(5,0,0,0) + 28541778720*B4(4,1,0,0) + 57666042720*B4(4,0,1,0) + 79064909000*B4(4,0,0,2) + 47705544432*B4(3,2,0,0) + 124943092560*B4(3,1,1,0) + 185011887060*B4(3,1,0,2) + 109041244416*B4(3,0,2,0) + 201831149520*B4(3,0,1,2) + 105721078320*B4(3,0,0,4) + 45247113912*B4(2,3,0,0) + 130596626160*B4(2,2,1,0) + 200545816240*B4(2,2,0,2) + 149592499056*B4(2,1,2,0) + 287145145980*B4(2,1,1,2) + 165757256280*B4(2,1,0,4) + 77170733640*B4(2,0,3,0) + 159980867046*B4(2,0,2,2) + 119713573980*B4(2,0,1,4) + 39178987848*B4(2,0,0,6) + 25463743760*B4(1,4,0,0) + 77170733640*B4(1,3,1,0) + 120922802000*B4(1,3,0,2) + 98217297360*B4(1,2,2,0) + 192377185000*B4(1,2,1,2) + 116604130500*B4(1,2,0,4) + 66881302488*B4(1,1,3,0) + 141479678340*B4(1,1,2,2) + 111162604410*B4(1,1,1,4) + 40422765240*B4(1,1,0,6) + 22243329108*B4(1,0,4,0) + 48795907160*B4(1,0,3,2) + 42251849640*B4(1,0,2,4) + 20165655510*B4(1,0,1,6) + 4577283711*B4(1,0,0,8) + 7422629760*B4(0,5,0,0) + 23143120000*B4(0,4,1,0) + 36717450000*B4(0,4,0,2) + 31104353280*B4(0,3,2,0) + 61685316000*B4(0,3,1,2) + 38465900000*B4(0,3,0,4) + 23631229440*B4(0,2,3,0) + 50614003440*B4(0,2,2,2) + 40913730000*B4(0,2,1,4) + 15710872320*B4(0,2,0,6) + 10406235840*B4(0,1,4,0) + 23113850760*B4(0,1,3,2) + 20590570000*B4(0,1,2,4) + 10377647280*B4(0,1,1,6) + 2628105480*B4(0,1,0,8) + 2207383360*B4(0,0,5,0) + 5002998000*B4(0,0,4,2) + 4679675000*B4(0,0,3,4) + 2620618000*B4(0,0,2,6) + 876035160*B4(0,0,1,8) + 140280140*B4(0,0,0,10) + 2387047520*B4(6,0,0,0) + 9309485328*B4(5,1,0,0) + 19042129080*B4(5,0,1,0) + 26257522720*B4(5,0,0,2) + 16112570760*B4(4,2,0,0) + 42722725500*B4(4,1,1,0) + 63623997360*B4(4,1,0,2) + 38084258160*B4(4,0,2,0) + 70895311344*B4(4,0,1,2) + 37595998440*B4(4,0,0,4) + 16217881680*B4(3,3,0,0) + 47389914000*B4(3,2,1,0) + 73188389120*B4(3,2,0,2) + 55446199380*B4(3,1,2,0) + 107038019088*B4(3,1,1,2) + 62554686480*B4(3,1,0,4) + 29618696250*B4(3,0,3,0) + 61752703320*B4(3,0,2,2) + 4678235100*B4(1,0,5,0) + 3414290880*B4(1,0,4,2) + 3233230000*B4(1,0,3,4) + 1849407560*B4(1,0,2,6) + 640179540*B4(1,0,1,8) + 108788680*B4(1,0,0,10) + 791494704*B4(0,6,0,0) + 2544090120*B4(0,5,1,0) + 4092771760*B4(0,5,0,2) + 3607982352*B4(0,4,2,0) + 7255368120*B4(0,4,1,2) + 4664165220*B4(0,4,0,4) + 3006651960*B4(0,3,3,0) + 6529831308*B4(0,3,2,2) + 5441526090*B4(0,3,1,4) + 2204878104*B4(0,3,0,6) + 1571349780*B4(0,2,4,0) + 3539043816*B4(0,2,3,2) + 3250142280*B4(0,2,2,4) + 1728484758*B4(0,2,1,6) + 480134655*B4(0,2,0,8) + 495213264*B4(0,1,5,0) + 1138096960*B4(0,1,4,2) + 1097450640*B4(0,1,3,4) + 648493560*B4(0,1,2,6) + 237780972*B4(0,1,1,8) + 45189144*B4(0,1,0,10) + 75741120*B4(0,0,6,0) + 176243760*B4(0,0,5,2) + 174845000*B4(0,0,4,4) + 109103280*B4(0,0,3,6) + 44633160*B4(0,0,2,8) + 11231220*B4(0,0,1,10) + 1361360*B4(0,0,0,12) + 445429920*B4(7,0,0,0) + 1762874568*B4(6,1,0,0) + 3637677680*B4(6,0,1,0) + 5036784480*B4(6,0,0,2) + 3122292420*B4(5,2,0,0) + 8351811000*B4(5,1,1,0) + 12489169680*B4(5,1,0,2) + 7555176720*B4(5,0,2,0) + 14122368792*B4(5,0,1,2) + 7555176720*B4(5,0,0,4) + 3259095840*B4(4,3,0,0) + 9607312000*B4(4,2,1,0) + 14898723840*B4(4,2,0,2) + 11406835440*B4(4,1,2,0) + 22111711776*B4(4,1,1,2) + 13036383360*B4(4,1,0,4) + 6235515000*B4(4,0,3,0) + 13054290480*B4(4,0,2,2) + 9976824000*B4(4,0,1,4) + 3384445680*B4(4,0,0,6) + 2182606608*B4(3,4,0,0) + 6755687120*B4(3,3,1,0) + 10690318080*B4(3,3,0,2) + 8912310316*B4(3,2,2,0) + 17628745680*B4(3,2,1,2) + 10913033040*B4(3,2,0,4) + 6430894470*B4(3,1,3,0) + 13738086648*B4(3,1,2,2) + 11024390520*B4(3,1,1,4) + 4155347196*B4(3,1,0,6) + 2353791440*B4(3,0,4,0) + 5214553344*B4(3,0,3,2) + 4611509760*B4(3,0,2,4) + 2281367088*B4(3,0,1,6) + 548725320*B4(3,0,0,8) + 947320920*B4(2,5,0,0) + 3016650000*B4(2,4,1,0) + 4833270000*B4(2,4,0,2) + 4202518320*B4(2,3,2,0) + 8416582020*B4(2,3,1,2) + 5360355000*B4(2,3,0,4) + 3383289000*B4(2,2,3,0) + 7317942570*B4(2,2,2,2) + 6041587500*B4(2,2,1,4) + 2404737720*B4(2,2,0,6) + 1639638000*B4(2,1,4,0) + 3677834160*B4(2,1,3,2) + 3346200000*B4(2,1,2,4) + 1748106360*B4(2,1,1,6) + 469111500*B4(2,1,0,8) + 413884380*B4(2,0,5,0) + 947320920*B4(2,0,4,2) + 904995000*B4(2,0,3,4) + 525314790*B4(2,0,2,6) + 186080895*B4(2,0,1,8) + 32792760*B4(2,0,0,10) + 251839224*B4(1,6,0,0) + 816621520*B4(1,5,1,0) + 1319157840*B4(1,5,0,2) + 1175249712*B4(1,4,2,0) + 2373100380*B4(1,4,1,2) + 1539017480*B4(1,4,0,4) + 1002217320*B4(1,3,3,0) + 2185604694*B4(1,3,2,2) + 1837398420*B4(1,3,1,4) + 755517672*B4(1,3,0,6) + 543182640*B4(1,2,4,0) + 1228428432*B4(1,2,3,2) + 1138096960*B4(1,2,2,4) + 614214216*B4(1,2,1,6) + 174594420*B4(1,2,0,8) + 181883884*B4(1,1,5,0) + 419732040*B4(1,1,4,2) + 408310760*B4(1,1,3,4) + 244843690*B4(1,1,2,6) + 91869921*B4(1,1,1,8) + 18106088*B4(1,1,0,10) + 31187520*B4(1,0,6,0) + 72870840*B4(1,0,5,2) + 72930000*B4(1,0,4,4) + 46181520*B4(1,0,3,6) + 19333080*B4(1,0,2,8) + 5045040*B4(1,0,1,10) + 649740*B4(1,0,0,12) + 33256080*B4(0,7,0,0) + 109174000*B4(0,6,1,0) + 177365760*B4(0,6,0,2) + 160485780*B4(0,5,2,0) + 325909584*B4(0,5,1,2) + 213981040*B4(0,5,0,4) + 141716250*B4(0,4,3,0) + 310816440*B4(0,4,2,2) + 264537000*B4(0,4,1,4) + 111105540*B4(0,4,0,6) + 81510000*B4(0,3,4,0) + 185391360*B4(0,3,3,2) + 173888000*B4(0,3,2,4) + 95855760*B4(0,3,1,6) + 28215000*B4(0,3,0,8) + 30568720*B4(0,2,5,0) + 70946304*B4(0,2,4,2) + 69871360*B4(0,2,3,4) + 42796208*B4(0,2,2,6) + 16628040*B4(0,2,1,8) + 3477760*B4(0,2,0,10) + 6961500*B4(0,1,6,0) + 16358760*B4(0,1,5,2) + 16575000*B4(0,1,4,4) + 10720710*B4(0,1,3,6) + 4647375*B4(0,1,2,8) + 1287000*B4(0,1,1,10) + 185640*B4(0,1,0,12) + 755820*B4(0,0,7,0) + 1790712*B4(0,0,6,2) + 1847560*B4(0,0,5,4) + 1234506*B4(0,0,4,6) + 566865*B4(0,0,3,8) + 175560*B4(0,0,2,10) + 33592*B4(0,0,1,12) + 3060*B4(0,0,0,14) + 37119160*B4(8,0,0,0) + 148476640*B4(7,1,0,0) + 308374560*B4(7,0,1,0) + 428298000*B4(7,0,0,2) + 267257952*B4(6,2,0,0) + 719540640*B4(6,1,1,0) + 1079310960*B4(6,1,0,2) + 657865728*B4(6,0,2,0) + 1233498240*B4(6,0,1,2) + 664191360*B4(6,0,0,4) + 285817532*B4(5,3,0,0) + 848030040*B4(5,2,1,0) + 1319157840*B4(5,2,0,2) + 1017636048*B4(5,1,2,0) + 1978736760*B4(5,1,1,2) + 1174195440*B4(5,1,0,4) + 565353360*B4(5,0,3,0) + 1187242056*B4(5,0,2,2) + 913263120*B4(5,0,1,4) + 313118784*B4(5,0,0,6) + 198696680*B4(4,4,0,0) + 619016580*B4(4,3,1,0) + 982566000*B4(4,3,0,2) + 825355440*B4(4,2,2,0) + 1637610000*B4(4,2,1,2) + 1020357000*B4(4,2,0,4) + 605260656*B4(4,1,3,0) + 1296987120*B4(4,1,2,2) + 1047566520*B4(4,1,1,4) + 399072960*B4(4,1,0,6) + 226972746*B4(4,0,4,0) + 504383880*B4(4,0,3,2) + 448957080*B4(4,0,2,4) + 224478540*B4(4,0,1,6) + 54872532*B4(4,0,0,8) + 91706160*B4(3,5,0,0) + 293930000*B4(3,4,1,0) + 472387500*B4(3,4,0,2) + 413853440*B4(3,3,2,0) + 831402000*B4(3,3,1,2) + 532950000*B4(3,3,0,4) + 338607360*B4(3,2,3,0) + 734657040*B4(3,2,2,2) + 610470000*B4(3,2,1,4) + 245583360*B4(3,2,0,6) + 168127960*B4(3,1,4,0) + 378287910*B4(3,1,3,2) + 346417500*B4(3,1,2,4) + 182908440*B4(3,1,1,6) + 49884120*B4(3,1,0,8) + 44054920*B4(3,0,5,0) + 101146500*B4(3,0,4,2) + 97256250*B4(3,0,3,4) + 57057000*B4(3,0,2,6) + 20540520*B4(3,0,1,8) + 3708705*B4(3,0,0,10) + 27350960*B4(2,6,0,0) + 89266320*B4(2,5,1,0) + 144644500*B4(2,5,0,2) + 129841920*B4(2,4,2,0) + 262990000*B4(2,4,1,2) + 171666000*B4(2,4,0,4) + 112529664*B4(2,3,3,0) + 246158640*B4(2,3,2,2) + 208288080*B4(2,3,1,4) + 86561280*B4(2,3,0,6) + 62486424*B4(2,2,4,0) + 141751610*B4(2,2,3,2) + 132182820*B4(2,2,2,4) + 72099720*B4(2,2,1,6) + 20828808*B4(2,2,0,8) + 21719880*B4(2,1,5,0) + 50277500*B4(2,1,4,2) + 49227750*B4(2,1,3,4) + 29835000*B4(2,1,2,6) + 11377080*B4(2,1,1,8) + 2297295*B4(2,1,0,10) + 3960320*B4(2,0,6,0) + 9282000*B4(2,0,5,2) + 9350000*B4(2,0,4,4) + 5984000*B4(2,0,3,6) + 2545920*B4(2,0,2,8) + 680680*B4(2,0,1,10) + 91000*B4(2,0,0,12) + 4803656*B4(1,7,0,0) + 15872220*B4(1,6,1,0) + 25865840*B4(1,6,0,2) + 23581584*B4(1,5,2,0) + 48036560*B4(1,5,1,2) + 31744440*B4(1,5,0,4) + 21162960*B4(1,4,3,0) + 46558512*B4(1,4,2,2) + 39884040*B4(1,4,1,4) + 16930368*B4(1,4,0,6) + 12471030*B4(1,3,4,0) + 28452424*B4(1,3,3,2) + 26860680*B4(1,3,2,4) + 14965236*B4(1,3,1,6) + 4476780*B4(1,3,0,8) + 4855032*B4(1,2,5,0) + 11302720*B4(1,2,4,2) + 11203920*B4(1,2,3,4) + 6935760*B4(1,2,2,6) + 2738736*B4(1,2,1,8) + 586872*B4(1,2,0,10) + 1175720*B4(1,1,6,0) + 2771340*B4(1,1,5,2) + 2826250*B4(1,1,4,4) + 1847560*B4(1,1,3,6) + 813960*B4(1,1,2,8) + 230945*B4(1,1,1,10) + 34580*B4(1,1,0,12) + 143208*B4(1,0,7,0) + 340340*B4(1,0,6,2) + 353430*B4(1,0,5,4) + 238680*B4(1,0,4,6) + 111384*B4(1,0,3,8) + 35343*B4(1,0,2,10) + 7020*B4(1,0,1,12) + 680*B4(1,0,0,14) + 379236*B4(0,8,0,0) + 1264120*B4(0,7,1,0) + 2068560*B4(0,7,0,2) + 1905904*B4(0,6,2,0) + 3898440*B4(0,6,1,2) + 2598960*B4(0,6,0,4) + 1750320*B4(0,5,3,0) + 3866616*B4(0,5,2,2) + 3341520*B4(0,5,1,4) + 1439424*B4(0,5,0,6) + 1069640*B4(0,4,4,0) + 2450448*B4(0,4,3,2) + 2333760*B4(0,4,2,4) + 1319472*B4(0,4,1,6) + 403920*B4(0,4,0,8) + 442442*B4(0,3,5,0) + 1034280*B4(0,3,4,2) + 1034280*B4(0,3,3,4) + 649740*B4(0,3,2,6) + 262548*B4(0,3,1,8) + 58344*B4(0,3,0,10) + 120120*B4(0,2,6,0) + 284310*B4(0,2,5,2) + 292500*B4(0,2,4,4) + 194040*B4(0,2,3,6) + 87480*B4(0,2,2,8) + 25740*B4(0,2,1,10) + 4095*B4(0,2,0,12) + 19448*B4(0,1,7,0) + 46410*B4(0,1,6,2) + 48620*B4(0,1,5,4) + 33320*B4(0,1,4,6) + 15912*B4(0,1,3,8) + 5236*B4(0,1,2,10) + 1105*B4(0,1,1,12) + 120*B4(0,1,0,14) + 1430*B4(0,0,8,0) + 3432*B4(0,0,7,2) + 3640*B4(0,0,6,4) + 2548*B4(0,0,5,6) + 1260*B4(0,0,4,8) + 440*B4(0,0,3,10) + 104*B4(0,0,2,12) + 15*B4(0,0,1,14) + B4(0,0,0,16)
Time: CPU 3.77 s, Wall: 3.77 s
sage: %timeit spin^(16)
5 loops, best of 3: 3.63 s per loop

I give this revised patch a positive review!

Anne

comment:15 Changed 9 years ago by jdemeyer

  • Milestone changed from sage-5.3 to sage-5.4

comment:16 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

1) There are issues with the documentation:

dochtml.log:/release/merger/sage-5.4.beta0/local/lib/python2.7/site-packages/sage/combinat/root_system/weyl_characters.py:docstring of sage.combinat.root_system.weyl_characters.WeylCharacterRing.dot_reduce:7: WARNING: Inline interpreted text or phrase reference start-string without end-string.
dochtml.log:/release/merger/sage-5.4.beta0/local/lib/python2.7/site-packages/sage/combinat/root_system/weyl_characters.py:docstring of sage.combinat.root_system.weyl_characters.WeightRing:0: WARNING: Block quote ends without a blank line; unexpected unindent.
dochtml.log:/release/merger/sage-5.4.beta0/local/lib/python2.7/site-packages/sage/combinat/root_system/weyl_characters.py:docstring of sage.combinat.root_system.weyl_characters.WeylCharacterRing.dot_reduce:11: WARNING: Inline interpreted text or phrase reference start-string without end-string.
dochtml.log:/release/merger/sage-5.4.beta0/local/lib/python2.7/site-packages/sage/combinat/root_system/weyl_characters.py:docstring of sage.combinat.root_system.weyl_characters.WeightRing:2: WARNING: Definition list ends without a blank line; unexpected unindent.
dochtml.log:/release/merger/sage-5.4.beta0/local/lib/python2.7/site-packages/sage/combinat/root_system/weyl_characters.py:docstring of sage.combinat.root_system.weyl_characters.WeylCharacterRing.dot_reduce:14: WARNING: Inline interpreted text or phrase reference start-string without end-string.
dochtml.log:/release/merger/sage-5.4.beta0/local/lib/python2.7/site-packages/sage/combinat/root_system/weyl_characters.py:docstring of sage.combinat.root_system.weyl_characters.WeightRing:5: WARNING: Block quote ends without a blank line; unexpected unindent.
dochtml.log:/release/merger/sage-5.4.beta0/local/lib/python2.7/site-packages/sage/combinat/root_system/weyl_characters.py:docstring of sage.combinat.root_system.weyl_characters.WeightRing:4: ERROR: Unexpected indentation.

2) The patch needs a proper commit message.

comment:17 Changed 9 years ago by bump

  • Status changed from needs_work to needs_review

I fixed the doc problem, and a bit of testing showed that it is probably a good idea to switch the two factors in a product if the second is bigger ... I'm not sure I found the best way to do it, but there's a new version of the patch, which now needs review again.

comment:18 follow-up: Changed 9 years ago by bump

  • Description modified (diff)

comment:19 in reply to: ↑ 18 Changed 9 years ago by aschilling

Hi Dan,

I think in your commit message, you can delete the two lines

imported patch weyl_characters.patch
* * *

I ran

sage -coverage weyl_characters.py 
----------------------------------------------------------------------
weyl_characters.py
SCORE weyl_characters.py: 94% (63 of 67)

Missing doctests:
	 * __pow__(self, n):
	 * irreducible_character_freudenthal(hwv, L, debug=False):
	 * get_branching_rule(Rtype, Stype, rule):
	 * __classcall__(cls, parent, prefix=None):


Possibly wrong (function name doesn't occur in doctests):
	 * _repr_(self):
	 * product_on_basis(self, a, b):
	 * simple_coroots(self):
	 * _repr_(self):
	 * product_on_basis(self, a, b):
	 * _repr_term(self, t):

Since the method __pow__(self, n) is new, I suppose at least this method should have a doctest. You can add #indirect doctest behind tests that indirectly test the function. Then sage -coverage probably won't complain about the possibly wrong functions any longer.

In the docstring for the method dot_reduce, I think you want the line

"Auxiliary function for product_on_basis."

as the first line. Then INPUT: and the - a ... with the same indent at INPUT.

Anne

comment:20 follow-up: Changed 9 years ago by bump

I made the changes Anne found, and doctest coverage is now 100 percent.

comment:21 in reply to: ↑ 20 Changed 9 years ago by aschilling

Replying to bump:

I made the changes Anne found, and doctest coverage is now 100 percent.

Thank you, Dan! I wrote a review patch with mostly trivial doctest changes. If you are happy with it, please fold it into yours and post a positive review on my behalf!

Anne

comment:22 Changed 9 years ago by bump

  • Description modified (diff)

comment:23 Changed 9 years ago by bump

I folded trac_13391-review-as.patch into trac_13391-weyl_characters.patch and uploaded.

Version 0, edited 9 years ago by bump (next)

comment:24 Changed 9 years ago by bump

  • Status changed from needs_review to positive_review

comment:25 Changed 9 years ago by bump

  • Status changed from positive_review to needs_work

I'm changing the status from positive_review to needs_work because I'm planning on adding two methods for the symmetric and exterior powers of representations. These can go in this patch.

comment:26 Changed 9 years ago by bump

  • Status changed from needs_work to positive_review

I've posted a revised patch which includes reasonably fast methods for symmetric and exterior powers, and adams operations. It seems possible that the new methods can be made faster and am looking for speedups.

comment:27 Changed 9 years ago by bump

  • Status changed from positive_review to needs_work

comment:28 follow-up: Changed 9 years ago by bump

  • Status changed from needs_work to needs_review

comment:29 in reply to: ↑ 28 Changed 9 years ago by aschilling

Hi Dan,

I posted a new review patch (with changes to the documentation) that you can fold into your patch if you agree with the changes. The doctests pass! I am happy to set a positive review.

Anne

comment:30 Changed 9 years ago by bump

The latest posted version contains the comments in trac_13391-review-as.patch as well as revision to the Lie thematic tutorial and some changes suggested by Nicolas.

comment:31 Changed 9 years ago by chapoton

Why does nobody care about the red light given by the bot ?

comment:32 Changed 9 years ago by bump

Apply trac_13391-weyl_characters.patch

comment:33 follow-up: Changed 9 years ago by bump

Why does nobody care about the red light given by the bot ?

The buildbot tried to apply the review patch, which was folded. According to the wiki, comment:32 might fix this?

comment:34 Changed 9 years ago by bump

  • Status changed from needs_review to needs_work

comment:35 in reply to: ↑ 33 Changed 9 years ago by nthiery

Replying to bump:

Why does nobody care about the red light given by the bot ?

The buildbot tried to apply the review patch, which was folded. According to the wiki, comment:32 might fix this?

Should I delete the reviewer's patch?

comment:36 Changed 9 years ago by bump

Should I delete the reviewer's patch?

adding comment:32 convinced the buildbot not to try the reviewer's patch, but after uploading the main patch again, it forgot. So unless the patch is deleted we need to add a message like comment:32 or comment:37 every time the patch is reposted. I don't mind doing this.

Last edited 9 years ago by bump (previous) (diff)

comment:37 Changed 9 years ago by bump

Apply trac_13391-weyl_characters.patch

comment:38 follow-up: Changed 9 years ago by bump

  • Status changed from needs_work to needs_review

comment:39 in reply to: ↑ 38 Changed 9 years ago by aschilling

Hi Dan,

Looks good! I have another small review patch, which I will e-mail in order not to confuse the buildbot. If you agree with it and fold it, you can set a positive review on my behalf. With it, all doctests pass and the documentation looks good.

Thanks,

Anne

Changed 9 years ago by bump

Weyl Character Ring speedup of product_on_basis

comment:40 Changed 9 years ago by bump

  • Status changed from needs_review to positive_review

I've applied the changes mentioned in comment:38, and the buildbot gives green. Therefore I'm setting positive review.

comment:41 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.4.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.