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: sage-9.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:

Status badges

Description (last modified by Michael Jung)

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 Frédéric Chapoton

so what ?

comment:2 Changed 2 years ago by Michael Jung

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 Michael Jung

Cc: Travis Scrimshaw Matthias Köppe added
Component: PLEASE CHANGEcombinatorics
Description: modified (diff)
Summary: Replace Assertion ErrorInfinite WeightedIntegerVectors does not coerce correctly
Type: PLEASE CHANGEdefect

comment:4 Changed 2 years ago by Michael Jung

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 Michael Jung

Summary: Infinite WeightedIntegerVectors does not coerce correctlyInfinite WeightedIntegerVectors does not coerce properly

comment:6 Changed 2 years ago by Michael Jung

Branch: u/gh-mjungmath/infinite_weightedintegervectors_does_not_coerce_properly

comment:7 Changed 2 years ago by Michael Jung

Commit: 4b7b8680c7d5dee0f4d210685f11f65809ef8ae0
Status: newneeds_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:

4b7b868Trac #30516: grading check before conversion

comment:8 Changed 2 years ago by Michael Jung

Authors: Michael Jung

comment:9 Changed 2 years ago by git

Commit: 4b7b8680c7d5dee0f4d210685f11f65809ef8ae0f698d3439b1407ee2801be69566b8c1f441d758d

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

f698d34Trac #30516: redundant line removed + doctest improved

comment:10 Changed 2 years ago by Michael Jung

Turns out the doctest example was already wrong...

comment:11 Changed 2 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw
Status: needs_reviewpositive_review

LGTM. The containment check should be quick IMO too.

comment:12 Changed 2 years ago by Volker Braun

Branch: u/gh-mjungmath/infinite_weightedintegervectors_does_not_coerce_properlyf698d3439b1407ee2801be69566b8c1f441d758d
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.