Opened 5 years ago

Closed 5 years ago

Last modified 4 years ago

#17977 closed defect (fixed)

Corner sum matrix ASM method has incorrect output

Reported by: jessicapalencia Owned by:
Priority: major Milestone: sage-6.6
Component: combinatorics Keywords: days64
Cc: tscrim, jamespropp, vinceknight, jcampbell, egunawan Merged in:
Authors: Jessica Striker, James Campbell, Kevin Dilks Reviewers: James Campbell, Vince Knight
Report Upstream: N/A Work issues:
Branch: 7f09e7e (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by tscrim)

Corner sum matrix method on AlternatingSignMatrix has incorrect output:

See the following example:

a = AlternatingSignMatrix([[0,0,1],[1,0,0],[0,1,0]])
a.corner_sum_matrix()
[0 0 0 0]
[0 0 1 1]
[0 0 1 2]
[0 1 2 3]

The result should be the transpose of what is displayed. See the definition in Section 2 of http://arxiv.org/abs/math/0208125.

[0 0 0 0]
[0 0 0 1]
[0 1 1 2]
[0 1 2 3]

Also, the documentation has this example, which is wrong and should be changed.

Change History (20)

comment:1 Changed 5 years ago by jessicapalencia

  • Description modified (diff)

comment:2 Changed 5 years ago by jessicapalencia

  • Description modified (diff)

comment:3 Changed 5 years ago by vinceknight

  • Cc vinceknight added

comment:4 Changed 5 years ago by tscrim

  • Description modified (diff)

comment:5 Changed 5 years ago by jcampbell

  • Cc jcampbell added

comment:6 Changed 5 years ago by egunawan

  • Cc egunawan added

comment:7 Changed 5 years ago by kdilks

  • Branch set to u/kdilks/asmpatch

comment:8 Changed 5 years ago by jcampbell

  • Commit set to 028e5f75cba440c4435ce8082a101a47d057748f

I'll look at this now


New commits:

028e5f7corrected code and incorrect example

comment:9 Changed 5 years ago by jcampbell

  • Status changed from new to needs_review

comment:10 Changed 5 years ago by jcampbell

  • Reviewers set to jcampbell
  • Status changed from needs_review to positive_review

all looks good to me

comment:11 Changed 5 years ago by jessicapalencia

Did all the tests pass? There are some functions that use corner_sum_matrix, such as gyration and height _function.

comment:12 Changed 5 years ago by jessicapalencia

And ASM_compatible_bigger, etc...

comment:13 Changed 5 years ago by jcampbell

all the tests in alternating_sign_matrix.py passed. Is it used in any other files?

comment:14 Changed 5 years ago by jcampbell

  • Status changed from positive_review to needs_work

More tests should be added to check non-symmetric cases.

comment:15 Changed 5 years ago by jcampbell

  • Authors changed from Jessica Striker to Jessica Striker, jcampbell
  • Branch changed from u/kdilks/asmpatch to u/jcampbell/17977
  • Commit changed from 028e5f75cba440c4435ce8082a101a47d057748f to 7f09e7eb242101e2963a50cd83334d5b02e096d5
  • Status changed from needs_work to needs_review

New commits:

7a4c6e3Merge branch 'u/kdilks/asmpatch' of trac.sagemath.org:sage into 17977
7f09e7e17977: adds some non-symmetric tests

comment:16 Changed 5 years ago by jcampbell

  • Authors changed from Jessica Striker, jcampbell to Jessica Striker, jcampbell, kdilks

comment:17 Changed 5 years ago by vinceknight

  • Reviewers changed from jcampbell to jcampbell, vinceknight
  • Status changed from needs_review to positive_review

I've checked through and the extra tests are good. As long as the buildbot passes I think this is now complete.

comment:18 Changed 5 years ago by jcampbell

  • Authors changed from Jessica Striker, jcampbell, kdilks to Jessica Striker, James Campbell, kdilks
  • Reviewers changed from jcampbell, vinceknight to James Campbell, Vince Knight

comment:19 Changed 5 years ago by vbraun

  • Branch changed from u/jcampbell/17977 to 7f09e7eb242101e2963a50cd83334d5b02e096d5
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:20 Changed 4 years ago by kcrisman

  • Authors changed from Jessica Striker, James Campbell, kdilks to Jessica Striker, James Campbell, Kevin Dilks
  • Commit 7f09e7eb242101e2963a50cd83334d5b02e096d5 deleted
Note: See TracTickets for help on using tickets.