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:  sagecombinat 

Priority:  major  Milestone:  sage9.4 
Component:  combinatorics  Keywords:  combinatorial free module 
Cc:  mjo, sagecombinat, nthiery, ghseeli  Merged in:  
Authors:  Travis Scrimshaw  Reviewers:  
Report Upstream:  N/A  Work issues:  
Branch:  public/combinat/fix_CFM_basis18750 (Commits, GitHub, GitLab)  Commit:  db219a19ca7a816249857c1fe58888b0c00fdcc5 
Dependencies:  Stopgaps: 
Description (last modified by )
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:
 #13776  fix Element Constructor in CombinatorialFreeModule?
Change History (10)
comment:1 Changed 6 years ago by
 Branch set to public/combinat/fix_CFM_basis18750
 Commit set to f697858783489df6b529d74fcd48a68235ef0d22
 Description modified (diff)
 Status changed from new to needs_review
comment:2 Changed 6 years ago by
 Status changed from needs_review to needs_work
Hmmm...this approach doesn't work when the index set is not callable/parentwithelements. 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
 Description modified (diff)
comment:4 Changed 6 years ago by
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
 Commit changed from f697858783489df6b529d74fcd48a68235ef0d22 to db219a19ca7a816249857c1fe58888b0c00fdcc5
Branch pushed to git repo; I updated commit sha1. New commits:
db219a1  Merge branch 'public/combinat/fix_CFM_basis18750' of trac.sagemath.org:sage into public/combinat/fix_CFM_basis18750

comment:6 Changed 5 years ago by
 Milestone changed from sage6.8 to sage7.3
comment:7 Changed 10 months ago by
 Milestone changed from sage7.3 to sage9.2
comment:8 Changed 9 months ago by
 Description modified (diff)
 Milestone changed from sage9.2 to sage9.3
comment:9 Changed 3 months ago by
 Milestone changed from sage9.3 to sage9.4
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:10 Changed 2 months ago by
 Cc mjo added
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:
Before:
New commits:
Make sure we always have an element of the basis indexing set.