Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#17637 closed defect (fixed)

Stopgap for IntegerListsLex

Reported by: ncohen Owned by:
Priority: blocker Milestone: sage-6.5
Component: combinatorics Keywords:
Cc: Merged in:
Authors: Nathann Cohen Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 5f00624 (Commits) Commit:
Dependencies: Stopgaps:

Description

As reported on #17548, the class IntegerListsLex returns wrong result. Until this is fixed, we need a stopgap to warn users.

Change History (12)

comment:1 Changed 5 years ago by ncohen

  • Branch set to public/17637
  • Priority changed from major to blocker
  • Status changed from new to needs_review

comment:2 Changed 5 years ago by git

  • Commit set to 5f0062460c512ebf001e12632daadb25961e7c81

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

5f00624trac #17637: Stopgap for IntegerListsLex

comment:3 follow-up: Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Since #17548 is about min_slope, shouldn't the warning be only if the min_slope argument was given?

comment:4 in reply to: ↑ 3 Changed 5 years ago by ncohen

  • Status changed from needs_info to needs_review

Since #17548 is about min_slope, shouldn't the warning be only if the min_slope argument was given?

In the code, it is said that the constraints are "assumed to be correct". I have no idea what happens if min_slope is not defined by max_slope is, or max_part or anything. If you believe that there is a situation in which the code will never return anything wrong you can adapt the error message.

Nathann

comment:5 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:6 Changed 5 years ago by ncohen

Thanks

Nathann

comment:7 Changed 5 years ago by vbraun

  • Status changed from positive_review to needs_work

Reviewer name missing

comment:8 Changed 5 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_work to positive_review

comment:9 follow-up: Changed 5 years ago by vbraun

  • Branch changed from public/17637 to 5f0062460c512ebf001e12632daadb25961e7c81
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:10 in reply to: ↑ 9 ; follow-up: Changed 5 years ago by aschilling

  • Commit 5f0062460c512ebf001e12632daadb25961e7c81 deleted

As far as I can see from #17548 there are actually no known bugs with regards to IntegerListsLex?. So I find it a total overreaction to put a stopgap as the limitations of IntegerListsLex? are correctly documented. Now warnings appear in many parts of the code which are completely unrelated to this issue (and as I said, there does not appear to be a bug). For example

sage: K = crystals.KirillovReshetikhin(['D',4,1],1,1)
/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/partition.py:4827:
********************************************************************************
This code contains bugs and may be mathematically unreliable.
This issue is being tracked at http://trac.sagemath.org/sage_trac/ticket/17548.
********************************************************************************

or

sage: Partitions(3,max_part=2)
/Applications/sage/local/lib/python2.7/site-packages/sage/combinat/partition.py:6622:
********************************************************************************
This code contains bugs and may be mathematically unreliable.
This issue is being tracked at http://trac.sagemath.org/sage_trac/ticket/17548.
********************************************************************************
Partitions of 3 having parts less than or equal to 2

both of which are totally fine!

Best,

Anne

comment:11 in reply to: ↑ 10 ; follow-up: Changed 5 years ago by ncohen

Dear Anne,

As far as I can see from #17548 there are actually no known bugs with regards to IntegerListsLex?.

What about those examples from #17548 ?

sage: Partitions(5, min_slope=1).list()
ValueError: [2, 4] is not a valid partition
sage: IntegerListsLex(5, length=3, max_slope=0).list()   # 0 is allowed in the partition
[[5, 0, 0], [4, 1, 0], [3, 2, 0], [3, 1, 1], [2, 2, 1]]
sage: IntegerListsLex(5, max_slope=0).list()             # now its not
[[5], [4, 1], [3, 2], [3, 1, 1], [2, 2, 1], [2, 1, 1, 1], [1, 1, 1, 1, 1]]

I will answer other points on #17898.

Nathann

comment:12 in reply to: ↑ 11 Changed 5 years ago by aschilling

Replying to ncohen:

Dear Anne,

As far as I can see from #17548 there are actually no known bugs with regards to IntegerListsLex?.

What about those examples from #17548 ?

sage: Partitions(5, min_slope=1).list()
ValueError: [2, 4] is not a valid partition
sage: IntegerListsLex(5, length=3, max_slope=0).list()   # 0 is allowed in the partition
[[5, 0, 0], [4, 1, 0], [3, 2, 0], [3, 1, 1], [2, 2, 1]]
sage: IntegerListsLex(5, max_slope=0).list()             # now its not
[[5], [4, 1], [3, 2], [3, 1, 1], [2, 2, 1], [2, 1, 1, 1], [1, 1, 1, 1, 1]]

I will answer other points on #17898.

Nathann

Read the answers that Travis gave on that ticket. They are consistent with the specifications or bad user input.

Anne

Note: See TracTickets for help on using tickets.