[qr_decomp] Some suggestion on the code an wording

[longye-tian]

Hi @jstac
I was reading through the qr_decomp lecture with codex and have some suggestions for the code and wording.
I've reviewed each of the suggestions myself.

I'd be grateful if you could take a look when you have a chance.

Matrix Factorization

• At lines 31–45, define the dimensions of a reduced QR decomposition and distinguish between an upper-triangular and an upper-trapezoidal matrix. For example, if A is an n × m matrix and k = min(n, m), then

$$ A = QR,\qquad Q \in \mathbb{R}^{n \times k},\qquad R \in \mathbb{R}^{k \times m},\qquad Q^\top Q = I_k. $$

Here, R is upper triangular when it is square and upper trapezoidal when it is rectangular, meaning that all entries below its main diagonal are zero.

• At line 42, use \top rather than T for the transpose.

• Change the section heading to sentence case: “Matrix factorization.”

• Add terminal punctuation to the sentences at lines 47 and 49.

Gram-Schmidt process

• At line 55, qualify the uniqueness claim. A nonsingular square matrix does not have a unique QR factorization without a sign convention. A standard convention is to require the diagonal entries of R to be positive.

• Apply the style-guide distinction between bold and italic consistently. Bold should identify terms being defined, while italics should indicate ordinary emphasis. For example, normalize and orthogonalize can remain bold if they are being introduced as defined steps, whereas words such as square, columns, and the later occurrences of eigenvalues should use italics or plain text.

• Remove math formatting from the prose term “QR” at line 55.

Gram-Schmidt process for square A

• At lines 128–131, correct the displayed definition of Q. It should contain the orthonormal vectors, without also being equated to the original columns of A:

$$ Q = \begin{bmatrix} e_1 & e_2 & \cdots & e_n \end{bmatrix}. $$

• Replace the Unicode middle dot used in mathematical expressions with \cdot.

• Replace the array and matrix environments used to display matrices with bmatrix, following the QuantEcon matrix-formatting convention.

• At line 120, correct “decomposision” to “decomposition.”

A not square

• Describe the resulting n × m matrix R as upper trapezoidal rather than upper triangular.

• Remove math formatting from the prose term “QR” at line 144.

Some Code

• At line 173, initialize the random number generator with a fixed seed so that all later examples are reproducible, for example:

rng = np.random.default_rng(1234)

• Rename QR_Decomposition to qr_decomposition and update its uses.

• The QR_Decomposition function at lines 177–199 supports square and wide matrices, where $m\ge n$, but not tall matrices, where $n>m$ . We could add a clarification sentence immediately before the function stating its supported dimensions and its assumption that the first n columns are linearly independent.

• At line 192, correct “vetor” to “vector.”

• At lines 206–235, revise the sign-normalization convention. Requiring a positive diagonal in Q is nonstandard, and diag_sign(Q) can destroy the factorization because np.sign(0) returns zero. For example:

A = np.array([
    [0.0, 1.0],
    [1.0, 0.0]
])

If a diagonal entry of Q is zero, the resulting diagonal sign matrix is singular and multiplication by it zeros a column of Q and a row of R. The usual convention is to choose signs from the diagonal of R, while mapping any zero sign to 1, so that the diagonal of R is nonnegative and the identity

$$ A = QR $$

is preserved.

• At line 206, remove the duplicated word in “There can be be sign differences.”

• Change the section heading to sentence case: “Some code.”

Example

• At lines 262–281, capitalize the package name consistently as “SciPy” when printed out or used in the text including the printed labels.

• The stopping and return logic in QR_eigvals at lines 337–350 can return incorrect eigenvalues. The routine treats a small change between consecutive iterates as convergence, but a stationary iterate need not be diagonal. For example, the matrix [[0, 1], [1, 0]] can remain unchanged, causing diff == 0, after which the function returns [0, 0] instead of [-1, 1]. The function also returns the diagonal unconditionally when maxiter is reached.

• At lines 334–343, convert the working matrix to floating-point storage. np.copy(A) preserves an integer input dtype, so later floating-point iterates are truncated during in-place assignment. For example:

A = np.array([
    [2, 1],
    [1, 2]
])

A suitable initialization would explicitly construct a floating-point working array.

• Remove the maintainer TODO at lines 322–324 because it is visible in the Colab version. When this section is revised, the numbered procedure could be migrated to a prf:algorithm directive.

• Replace the manually bolded remark at line 326 with a prf:remark directive. Its statement should also be revised alongside the decision about the algorithm’s intended scope.

• Rename QR_eigvals to qr_eigvals.

• At lines 353–369, either change the text to say NumPy The current code calls np.linalg.eigvals, not scipy.linalg.eigvals.

• Use a deterministic and deliberately supported example rather than an arbitrary random general matrix. Seeding the generator makes the example reproducible, but it does not ensure that the sampled matrix lies in the class the simplified algorithm can handle.

• Change the section heading to sentence case: “Using QR decomposition to compute eigenvalues.”

• Remove math formatting from the prose term “QR” at line 314.

QR and PCA

• Remove math formatting from “QR” in the heading and at line 374. The heading should be plain text: “QR and PCA.”

• Replace prime notation for matrix transposes with \top throughout this section.

• At lines 378–380, use the standard normal-distribution notation

$$ N(\mu, \Sigma) $$

rather than a calligraphic N, and replace n > > k with

$$ n \gg k. $$

according to the style guide.

• At line 389, remove the duplicated “that.”

• At line 407, correct the code comment. rng.multivariate_normal(..., size=n) returns an n × k array in which each row, not each column, is a multivariate-normal draw.

• At lines 451–459, replace the diagonal-based eigenvector sign adjustment. diag_sign can zero a column when an eigenvector has a zero diagonal entry, and separate diagonal signs do not reliably align corresponding eigenvectors. A sign-invariant comparison could instead inspect

np.abs(np.diag(P.T @ QP_tilde))

What do you think? Happy to put up a PR.

Best,
Longye

[jstac]

Thanks @longye-tian , great suggestions. Please go ahead!