Opened 8 years ago

Closed 8 years ago

#14350 closed enhancement (fixed)

Implement multiplicative inverses of cycle index series

Reported by: agd Owned by: agd
Priority: major Milestone: sage-5.11
Component: combinatorics Keywords: species, cycle index
Cc: sage-combinat, MartinRubey Merged in: sage-5.11.beta1
Authors: Andrew Gainer-Dewar Reviewers: Frédéric Chapoton
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by chapoton)

In the context of virtual species (see #14348), it is also possible to define multiplicative inverses of many species. This patch incorporates code to compute the cycle indices of these multiplicative inverse species and a div method for cycle index series.

Apply trac_14350_cycle_index_mult_inverses.patch

Attachments (3)

cycle_index_mult_inverses-agd.2.patch (2.0 KB) - added by agd 8 years ago.
Rename inversion function to inverse
cycle_index_mult_inverses-agd.patch (2.0 KB) - added by agd 8 years ago.
Rename inversion function to invert
trac_14350_cycle_index_mult_inverses.patch (2.3 KB) - added by agd 8 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 follow-up: Changed 8 years ago by agd

If someone with permissions to do so could delete the ".path" file I accidentally attached, that would be swell.

comment:2 in reply to: ↑ 1 Changed 8 years ago by nthiery

Replying to agd:

If someone with permissions to do so could delete the ".path" file I accidentally attached, that would be swell.

Done.

comment:3 Changed 8 years ago by nthiery

You probably want to call your method invert (see http://docs.python.org/2/reference/datamodel.html). And then it's likely that div's default implementation will do exactly what you want.

Other than this, you might want to ask MartinRubey? to review this patch!

Cheers,

Nicolas

comment:4 Changed 8 years ago by agd

Nicolas,

invert looks like it should be exactly what's needed here, but it doesn't seem to work; Sage doesn't seem to know to try to use it for div. My guess is that there's some additional step I'd need to take to tell Sage that CycleIndexSeriesRing? now (usually) has division, but I'm not sure what that is. (For this purpose, is it likely to be a problem that not every element has a multiplicative inverse?)

Thanks again for your help!

--Andrew

Changed 8 years ago by agd

Rename inversion function to inverse

comment:5 Changed 8 years ago by agd

I've renamed the cycle index multiplicative inversion method as invert as per Nicolas' suggestion. However, I haven't been able to find a way to convince Sage to use this method on its own, so the patch still includes a thin _div_ method as well.

(Also, once again, I've made a mess of the attachments. Could the next person through with privileges to do so please remove --.2.patch? Thanks!)

comment:6 Changed 8 years ago by agd

  • Cc MartinRubey added

comment:7 follow-up: Changed 8 years ago by agd

Per Nicolas' suggestion, I've CC'ed Martin Rubey on this ticket. (If this is not the correct way to bring it to your attention, I am very sorry! I'm still very new to this.)

comment:8 in reply to: ↑ 7 Changed 8 years ago by nthiery

Replying to agd:

Per Nicolas' suggestion, I've CC'ed Martin Rubey on this ticket. (If this is not the correct way to bring it to your attention, I am very sorry! I'm still very new to this.)

That usually works. I am not sure what his login is though; you might want to make a quick search (in principle the list of login's in on trac's main web page, but I haven't found him there).

A private e-mail can increase the chances of success :-) In this case you probably want to send an e-mail on sage-combinat-devel for advertising your work.

Cheers,

Nicolas

comment:9 Changed 8 years ago by mhansen

I'll take a look at this.

Changed 8 years ago by agd

Rename inversion function to invert

comment:10 Changed 8 years ago by agd

  • Status changed from new to needs_review

Per discussions elsewhere, I've uploaded a new version of the patch which was generated using "hg export tip". The content is the same.

comment:11 follow-up: Changed 8 years ago by chapoton

  • Description modified (diff)

for the bot: apply trac_14350_cycle_index_mult_inverses.patch

You should use python3 syntax for raise:

raise ValueError("constant term must be non-zero")

you could avoid to define parent by using directly self.parent()

you should doctest the _div_ function

comment:12 in reply to: ↑ 11 Changed 8 years ago by agd

Replying to chapoton:

You should use python3 syntax for raise:

raise ValueError("constant term must be non-zero")

you could avoid to define parent by using directly self.parent()

you should doctest the _div_ function

I have uploaded a new patch which makes these changes. Thanks for the review!

comment:13 Changed 8 years ago by chapoton

for the bot: apply trac_14350_cycle_index_mult_inverses.patch

comment:14 Changed 8 years ago by chapoton

ok, looks good to me, positive review.

comment:15 Changed 8 years ago by mhansen

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

comment:16 Changed 8 years ago by jdemeyer

  • Milestone changed from sage-5.10 to sage-5.11

comment:17 Changed 8 years ago by jdemeyer

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