Opened 6 years ago

Last modified 2 months ago

#18750 needs_work defect

Make sure indices converted into the index set of the basis for a CombinatorialFreeModule

Reported by: tscrim Owned by: sage-combinat
Priority: major Milestone: sage-9.4
Component: combinatorics Keywords: combinatorial free module
Cc: mjo, sage-combinat, nthiery, ghseeli Merged in:
Authors: Travis Scrimshaw Reviewers:
Report Upstream: N/A Work issues:
Branch: public/combinat/fix_CFM_basis-18750 (Commits, GitHub, GitLab) Commit: db219a19ca7a816249857c1fe58888b0c00fdcc5
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

The following is unexpected:

sage: F = CombinatorialFreeModule(QQ, Partitions())
sage: F.monomial((2,1,1))
B[(2, 1, 1)]

We should make sure that we always have the support of an element of the indexing set for a CombinatorialFreeModule.

We also need a similar fix for term.

Related:

Change History (10)

comment:1 Changed 6 years ago by tscrim

  • Branch set to public/combinat/fix_CFM_basis-18750
  • Commit set to f697858783489df6b529d74fcd48a68235ef0d22
  • Description modified (diff)
  • Status changed from new to needs_review

We loose about half a microsecond each time this is called, but which can add a little bit of slowdown. However I think this is one which we must accept (and will typically be negligible due to caching or other parts of the computation). Although in these examples, we seem to achieve a marginal net speedup. So I think this is a good solution (but I haven't done extensive testing).

With:

sage: s = SymmetricFunctions(QQ).s()
sage: %timeit s[5,3,1,1] * s[3,3,2]
100 loops, best of 3: 15.2 ms per loop
sage: I = DescentAlgebra(QQ, 4).I()
sage: timeit('I[3,1] * I[2,2]', number=10)
10 loops, best of 3: 223 ms per loop

Before:

sage: s = SymmetricFunctions(QQ).s()
sage: %timeit s[5,3,1,1] * s[3,3,2]
100 loops, best of 3: 15.7 ms per loop
sage: I = DescentAlgebra(QQ, 4).I()
sage: timeit('I[3,1] * I[2,2]', number=10)
10 loops, best of 3: 225 ms per loop

New commits:

f697858Make sure we always have an element of the basis indexing set.

comment:2 Changed 6 years ago by tscrim

  • Status changed from needs_review to needs_work

Hmmm...this approach doesn't work when the index set is not callable/parent-with-elements. I'd prefer not to change the logic of Family, but I'm wondering if that's what we have to do...

comment:3 Changed 6 years ago by tscrim

  • Description modified (diff)

comment:4 Changed 6 years ago by nthiery

Very briefly for now: +1 to improving term/monomial in this direction (with a check=True/False? additional argument).

Cheers,

comment:5 Changed 5 years ago by git

  • Commit changed from f697858783489df6b529d74fcd48a68235ef0d22 to db219a19ca7a816249857c1fe58888b0c00fdcc5

Branch pushed to git repo; I updated commit sha1. New commits:

db219a1Merge branch 'public/combinat/fix_CFM_basis-18750' of trac.sagemath.org:sage into public/combinat/fix_CFM_basis-18750

comment:6 Changed 5 years ago by tscrim

  • Milestone changed from sage-6.8 to sage-7.3

comment:7 Changed 10 months ago by mkoeppe

  • Milestone changed from sage-7.3 to sage-9.2

comment:8 Changed 9 months ago by mkoeppe

  • Description modified (diff)
  • Milestone changed from sage-9.2 to sage-9.3

comment:9 Changed 3 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:10 Changed 2 months ago by mjo

  • Cc mjo added
Note: See TracTickets for help on using tickets.