Opened 2 years ago
Closed 2 years ago
#30516 closed defect (fixed)
Infinite WeightedIntegerVectors does not coerce properly
Reported by:  Michael Jung  Owned by:  

Priority:  major  Milestone:  sage9.2 
Component:  combinatorics  Keywords:  
Cc:  Travis Scrimshaw, Matthias Köppe  Merged in:  
Authors:  Michael Jung  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  f698d34 (Commits, GitHub, GitLab)  Commit:  f698d3439b1407ee2801be69566b8c1f441d758d 
Dependencies:  Stopgaps: 
Description (last modified by )
Today, I have encountered the following bug:
sage: W = WeightedIntegerVectors([1,2,3]) sage: w = W([1,1,1]) sage: parent(w) Integer vectors of 0 weighted by [1, 2, 3]
This is definitely wrong. The parent should be Integer vectors of 6 weighted by [1, 2, 3]
.
Change History (12)
comment:1 Changed 2 years ago by
comment:2 Changed 2 years ago by
I created this ticket accidentally. Sorry. The task has been finished in #30275.
I will replace the name and description of this ticket soon. There are still open tasks.
comment:3 Changed 2 years ago by
Cc:  Travis Scrimshaw Matthias Köppe added 

Component:  PLEASE CHANGE → combinatorics 
Description:  modified (diff) 
Summary:  Replace Assertion Error → Infinite WeightedIntegerVectors does not coerce correctly 
Type:  PLEASE CHANGE → defect 
comment:4 Changed 2 years ago by
Any ideas where this bug is originated? I would say it comes from:
sage: W0 = WeightedIntegerVectors(0, [1,2,3]) sage: W0([1,1,1])
This is a bug, too. As far as I understand the code of DisjointUnionEnumeratedSets
, it checks all subsets until the element is coercible into this subset.
Probably a more direct approach is more useful and faster here, i.e. compute the grading (already there) and coerce into the corresponding weighted integer vector.
Opinions?
comment:5 Changed 2 years ago by
Summary:  Infinite WeightedIntegerVectors does not coerce correctly → Infinite WeightedIntegerVectors does not coerce properly 

comment:6 Changed 2 years ago by
Branch:  → u/ghmjungmath/infinite_weightedintegervectors_does_not_coerce_properly 

comment:7 Changed 2 years ago by
Commit:  → 4b7b8680c7d5dee0f4d210685f11f65809ef8ae0 

Status:  new → needs_review 
I think, this should solve the problem at hand.
As mentioned, one can still improve the performance a bit. But let's spare it for another ticket.
New commits:
4b7b868  Trac #30516: grading check before conversion

comment:8 Changed 2 years ago by
Authors:  → Michael Jung 

comment:9 Changed 2 years ago by
Commit:  4b7b8680c7d5dee0f4d210685f11f65809ef8ae0 → f698d3439b1407ee2801be69566b8c1f441d758d 

Branch pushed to git repo; I updated commit sha1. New commits:
f698d34  Trac #30516: redundant line removed + doctest improved

comment:11 Changed 2 years ago by
Reviewers:  → Travis Scrimshaw 

Status:  needs_review → positive_review 
LGTM. The containment check should be quick IMO too.
comment:12 Changed 2 years ago by
Branch:  u/ghmjungmath/infinite_weightedintegervectors_does_not_coerce_properly → f698d3439b1407ee2801be69566b8c1f441d758d 

Resolution:  → fixed 
Status:  positive_review → closed 
so what ?