Ticket #2299 (closed enhancement: fixed)

Opened 5 years ago

Last modified 5 years ago

[with patch, positive review] add zero_matrix constructor.

Reported by: ncalexan Owned by: was
Priority: minor Milestone: sage-2.10.3
Component: linear algebra Keywords: zero matrix identity one
Cc: ncalexan Work issues:
Report Upstream: Reviewers:
Authors: Merged in:
Dependencies: Stopgaps:

Description

This adds the zero_matrix convenience constructor, just like identity_matrix.

Attachments

2299-ncalexan-zero-matrix-1.patch Download (2.8 KB) - added by ncalexan 5 years ago.
2299-ncalexan-zero-matrix-2.patch Download (2.6 KB) - added by ncalexan 5 years ago.
2299.patch Download (3.3 KB) - added by mhansen 5 years ago.

Change History

Changed 5 years ago by ncalexan

comment:1 Changed 5 years ago by malb

  • Owner changed from malb to was
  • Component changed from commutative algebra to linear algebra

You replaced "n x n" by "$n \times n$" which reads nicer if typesetted. However, it is harder to comprehend from the command line (foo?).

I am not sure that we need identity_matrix and zero_matrix (i.e. the functional versions of MS(0) and MS(1)) but that is a matter of taste.

comment:2 Changed 5 years ago by ncalexan

I really want identity_matrix and zero_matrix because I often don't want to name the matrix space in advance. MS(0) requires me to create said space first.

As for latex in docstrings... I don't care either way. I think it should be latex but I parse simple latex from tex without trouble.

comment:3 Changed 5 years ago by was

Just a quick remark. You can make a zero matrix using the matrix function:

sage: matrix(ZZ, 2,3)
[0 0 0]
[0 0 0]

I don't think that's a good reason not to add zero_matrix and identity_matrix functions though, both of which I would like to have too.

comment:4 Changed 5 years ago by ncalexan

I think zero_ and identity_ declare programmer intent nicely. Will someone say positive or negative and this either goes to the bit-bucket or gets applied?

comment:5 Changed 5 years ago by was

  • Summary changed from [with patch, needs review] add zero_matrix constructor. to [with patch, needs update] add zero_matrix constructor.

REFEREE REPORT:

I definitely think these functions should go in, but with TWO caveats.

  1. I think the zero_matrix should have an option to be nonsquare though. Please post another patch that adds that, so, e.g.,
sage: zero_matrix(ZZ, 2,3)
[0 0 0]
[0 0 0]

works.

  1. There should be a sparse option for both zero_matrix and

identity_matrix.

sage: zero_matrix(ZZ, 2,3, sparse=True).parent()
Full MatrixSpace of 2 by 3 sparse matrices over Integer Ring

-- William

comment:6 Changed 5 years ago by was

  • Summary changed from [with patch, needs update] add zero_matrix constructor. to [with patch, positive review pending updates] add zero_matrix constructor.

Changed 5 years ago by ncalexan

comment:7 Changed 5 years ago by ncalexan

2299-ncalexan-zero-matrix-2.patch should apply without the previous patch, and should address the referee's comments.

I generated this patch using hg diff between two revisions, not hg export. Let me know if it's not okay.

comment:8 Changed 5 years ago by mhansen

  • Summary changed from [with patch, positive review pending updates] add zero_matrix constructor. to [with patch, positive review] add zero_matrix constructor.

Works for me in 2.10.3.alpha0. Note that only the last patch should be apply and that it is a "raw" patch.

Changed 5 years ago by mhansen

comment:9 Changed 5 years ago by mhansen

I just realized this conflicts with #874. #874 should be applied first, and then 2299.patch should be applied.

comment:10 Changed 5 years ago by mabshoff

  • Status changed from new to closed
  • Resolution set to fixed

Merged 2299.patch in Sage 2.10.3.rc0

Note: See TracTickets for help on using tickets.