More matrix features to MathObject matrices

[pstaabp]

This builds on #1012 and includes

• new methods for setting elements, removing columns and rows and a submatrix.
• A test suite for nearly all of the methods in the module.
• updated POD.

[Alex-Jordan]

I was looking at the POD since a lot of that is in the diff here, and I left some comments. But I'm going to pause because I think if you proofread the actual POD output you will see a lot of the same things that I'm seeing. I'll move on to testing the new tools now.

[Alex-Jordan]

This has some issues as well. If I try it on a 1D matrix, I get an error:

$A = Matrix([ 1, 2, 3, 4 ]);
$B = $A->subMatrix([3]);

And if I try it on a 3D matrix, it is not returning what I would expect it to:

$A = Matrix([ [[1,2],[3,4]], [[5,6], [7,8]]]);
$B = $A->subMatrix([1], [1,2], [2]);
# $B seems to be what I would expect from subMatrix([1], [1,2], [1,2]);

[Alex-Jordan]

(It did seem to work when I tested with 2D matrices though.)

[pstaabp]

I thought of subMatrix only on 2D matrices, so will rework for other dimensions.

[pstaabp]

Rewrote this now. Should be working on non-2D matrices.

[drgrice1]

This needs to be

Suggested change

	my $el = $self->{data}[ $ind->[0] - 1 ];
	for my $i (1 .. scalar(@$ind) - 1) { $el = $el->{data}[ $ind->[$i] - 1 ]; }
	# update the value of $el
	$el = Value::makeValue($value);
	my $el = \($self->{data}[ $ind->[0] - 1 ]);
	for my $i (1 .. scalar(@$ind) - 1) { $el = \($$el->{data}[ $ind->[$i] - 1 ]); }
	# update the value of $el
	$$el = Value::makeValue($value);

	my $el = \($self->{data}[ $ind->[0] - 1 ]);
	for my $i (1 .. scalar(@$ind) - 1) { $el = \($$el->{data}[ $ind->[$i] - 1 ]); }

	# update the value of $el
	$$el = Value::makeValue($value);

in order to actually mutate the matrix.

[pstaabp]

Changed this, but odd that the test works for the previous version of the code as well as this one, however, I think the test needs some help.

[Alex-Jordan]

See my other comment about column. Currently for a 1D matrix, $matrix->column(1) is a Real. I do think it should be changed to be a MathObject Matrix though.

With the element method, it is more complicated than this. If you have an nD matrix and you pass n whole numbers to element, then yes you get a Real (or Complex, I assume). But if you pass fewer than n whole numbers, you get a Matrix. (And if you pass more than n, you get null.)

[Alex-Jordan]

See my recent comments just now. I feel like we need to work on this file in a more methodical manner, especially since there are things to fix that precede the new things here. I propose:

1. Go through the current methods and (a) make them better if we see opportunity for that, and (b) correct their POD without moving POD around.
2. Then reorganize the POD and provide/update a test suite for the existing methods. The test suite should be thorough with at least 1D, 2D, and 3D matrices.
3. Then add new methods, with new POD, and new tests.

When I look at the diff for this PR, it's just too overwhelming to sort out what is just POD moving around and what is new content.

[pstaabp]

Would it make sense if we split this in two pieces?

1. update the POD for existing methods and functionality
2. Update the methods as you mentioned and with them any needed POD.

[pstaabp]

This is being rewritten using different PRs. The first one is #1197 which cleans up the POD and a second one will take some of the functionality in this PR. Once both are merged, we will close this one.

[Alex-Jordan]

We should catalog what is still here that needs to be added.

• setElement method
• setExtractElements method
• subMatrix method
• removeRow method
• removeColumn method

Did I miss anything? We should be careful to add them so that they work for a Matrix object of any degree. We should agree on what things like "removeRow" and "removeColumn" mean for degrees other than 2.

[somiaj]

I think the methods in ReduceMatrix.pl would be nice to be part of matrices by default.

[Alex-Jordan]

I've opened PRs that recover setElement (renamed as set and reworked) and subMatrix (nearly identical to as here). That leaves:

• removeRow
• removeColumn

and @somiaj also mentioned some methods from ReduceMatrix.pl. But regarding removeRow and removeColumn, I propose some conventions.

1. No matter what degree a matrix has, its "rows" are specified by the first index.
2. No matter what degree a matrix has, its "columns" are specified by the second index.

Good thing about this convention:

1. removeRow and removeColumn both have meaning for all degree matrices, except removeColumn doesn't make sense for degree 1 matrices.
2. These methods could be implemented without new code. Just call subMatrix in the appropriate way.

"Bad" thing about this convention:

1. For a degree 1 Matrix, it is presented as a row. In fact, the isRow method returns 1 if and only if the Matrix is degree 1. But I'm suggesting if $M is a degree 1 Matrix, then $M->removeRow(2) would remove its second entry. It would look like the second column is removed.

So an alternative would be to still make these methods illegal on degree 1 Matrices, yet still apply to degrees above 2.

[Alex-Jordan]

I think the methods in ReduceMatrix.pl would be nice to be part of matrices by default.

@somiaj Where is ReduceMatrix.pl? I don't find that in pg or the OPL.

[somiaj]

It was my dyslexia, sorry about that, it is macros/math/MatrixReduce.pl. One I use is rref to do things like grade if a matrix is in Echelon form.

[Alex-Jordan]

OK, I found MatrixReduce.pl.

Most things in there are now implemented in Matrix.pm, maybe with different syntax. One thing that is not is row reduction. But I'm not sure that should be added to Matrix.pm. If all entries of a matrix are integers, then you could program a row reduction algorithm in a way that avoided machine rounding issues only by (a) using the Fraction context or (b) saving all division until the last step in the algorithm.

But even then, what about when your matrix has floating point values in the first place? Well, if you think they all come from fractions, you could apply Fraction() to them first, again using the Fraction context.

But even then, what if floating point entries came from irrational numbers?

So I see too many floating point issues with doing a row reduction algorithm in Matrix.pm.

We could put something like is_rref though into Matrix.pm, just to check if a matrix is reduced or not.

[Alex-Jordan]

With #1421 and #1424 open now, I'm going to close this older PR. The conversation about rref can continue though.

[somiaj]

I haven't ran into enough floating point issues on smaller 3x3 or 4x4 matrices for the method in MatrixReduce.pl to be a problem. Basically I check for echelon form, by checking the two matrices rref matches and if the zeros are in the right spot of the matrix.

It would be nice to not have to use that older macro for this, and I think since these are math objects, equality isn't exact so floating point issues have never gotten bad enough to not mark answers correct.

[Alex-Jordan]

When you have used this, have you been also using Fraction context?

Based on your second paragraph, I'm not sure the issue is clear. It's not an issue about one value being slightly different from another value. In a row reduction algorithm, scaling and subtraction happens in a way that is engineered to make one element of a row become zero, while other elements in the row get the same treatment and may or may not come out as zero. When one of those other elements in that row should become zero, but actually comes out nonzero because of machine rounding, it throws a big wrench in the algorithm. The algorithm reaches a cell that mathematically should have come to zero, and the algorithm should have moved to look at a cell to the right. But since it reaches a nonzero cell (owing to the rounding error), it normalizes the entire row so that cell has value 1. So what should not have been a pivot position comes out as a pivot position.

Try this MWE:

DOCUMENT();

loadMacros(qw(PGstandard.pl PGML.pl MatrixReduce.pl));

Context("Matrix");
$A = Matrix([ 0.2, 0.6], [ 10000, 30000 ]);
$R = rref($A);

BEGIN_PGML
[`[$A]`] reduces to [`[$R]`]
END_PGML

ENDDOCUMENT();

That matrix should row reduce to have only one pivot position, but I'm getting it to come out as the identity matrix. And it's just the classic machine rounding issue with an rref algorithm.

[somiaj]

I haven't tested the fraction context, and yea my matrices are nice in that they won't create something that the issues can cause problems. I can keep loading this older macro, but it also means it is harder to deprecate.

To me I think it would be better to have the rref algorithm available with maybe a warning, and let authors decide if it is useful or not. If there are ways to improve it, that is good to. I also have algorithms that I use to put matrices in echelon form with no division to avoid such issues, but these are for writing solutions not graders.