From 5e28302b4f3f5a23cb74d9812f8b51480d49d35c Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Wed, 7 Sep 2022 21:54:38 +0100 Subject: [PATCH 01/14] Used a message that performs the same logic in its method. --- src/Math-Matrix/PMQRDecomposition.class.st | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index 387542b7..987e8aad 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -128,7 +128,7 @@ PMQRDecomposition >> decomposeWithPivot [ i := q numberOfColumns - i. pivot := pivot copyFrom: 1 to: i. q := PMMatrix rows: - (q rows collect: [ :row | row copyFrom: 1 to: i ]) ]. + (q rowsCollect: [ :row | row copyFrom: 1 to: i ]) ]. ^ Array with: q with: r with: pivot ] From 5c09925155ec840c0fabb902f2dc50290451286a Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Wed, 7 Sep 2022 21:55:26 +0100 Subject: [PATCH 02/14] Performed the same refactoring in the pivot message's method. --- src/Math-Matrix/PMQRDecomposition.class.st | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index 987e8aad..fad41fc7 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -61,7 +61,7 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections r := PMMatrix rows: (r rows copyFrom: 1 to: colSize). i := q numberOfColumns - i. q := PMMatrix rows: - (q rows collect: [ :row | row copyFrom: 1 to: i ]) ]. + (q rowsCollect: [ :row | row copyFrom: 1 to: i ]) ]. ^ Array with: q with: r ] From 3a04d724e3dce576320a553b7593bf38452b804a Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Wed, 7 Sep 2022 22:01:50 +0100 Subject: [PATCH 03/14] Extracted duplicate code to an intention revealing message and method. --- src/Math-Matrix/PMQRDecomposition.class.st | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index fad41fc7..868a34d8 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -58,7 +58,7 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections i := i + 1. colSize := colSize - 1 ]. i > 0 ifTrue: [ - r := PMMatrix rows: (r rows copyFrom: 1 to: colSize). + r := self upperTriangularPartOf: r With: colSize. i := q numberOfColumns - i. q := PMMatrix rows: (q rowsCollect: [ :row | row copyFrom: 1 to: i ]) ]. @@ -124,7 +124,7 @@ PMQRDecomposition >> decomposeWithPivot [ i := i + 1. colSize := colSize - 1 ]. i > 0 ifTrue: [ - r := PMMatrix rows: (r rows copyFrom: 1 to: colSize). + r := self upperTriangularPartOf: r With: colSize. i := q numberOfColumns - i. pivot := pivot copyFrom: 1 to: i. q := PMMatrix rows: @@ -158,3 +158,9 @@ PMQRDecomposition >> of: matrix [ r := self initialRMatrix. q := self initialQMatrix. ] + +{ #category : #private } +PMQRDecomposition >> upperTriangularPartOf: matrix With: columnSize [ + + ^ PMMatrix rows: (r rows copyFrom: 1 to: columnSize) +] From 97bc32bde43fe130517c042cd05d4d963e2a6aab Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Wed, 7 Sep 2022 22:37:59 +0100 Subject: [PATCH 04/14] Clarified the name of a temporary variable. --- src/Math-Matrix/PMQRDecomposition.class.st | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index 868a34d8..3de2c848 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -46,13 +46,13 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections (householderVector at: 1) * (householderVector at: 2) * matrixOfMinor). matrixOfMinor rowsWithIndexDo: [ :aRow :index | - aRow withIndexDo: [ :n :c | + aRow withIndexDo: [ :element :c | r rowAt: col + index - 1 columnAt: col + c - 1 - put: ((n closeTo: 0) + put: ((element closeTo: 0) ifTrue: [ 0 ] - ifFalse: [ n ]) ] ] ]. + ifFalse: [ element ]) ] ] ]. i := 0. [ (r rowAt: colSize) isZero ] whileTrue: [ i := i + 1. From df9874c328f3e934c85baf5f1b3d2dc4b1f3c700 Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Wed, 7 Sep 2022 22:43:54 +0100 Subject: [PATCH 05/14] Clarified the name of a block variable. --- src/Math-Matrix/PMQRDecomposition.class.st | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index 3de2c848..b443ec9d 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -46,10 +46,10 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections (householderVector at: 1) * (householderVector at: 2) * matrixOfMinor). matrixOfMinor rowsWithIndexDo: [ :aRow :index | - aRow withIndexDo: [ :element :c | + aRow withIndexDo: [ :element :column | r rowAt: col + index - 1 - columnAt: col + c - 1 + columnAt: col + column - 1 put: ((element closeTo: 0) ifTrue: [ 0 ] ifFalse: [ element ]) ] ] ]. From 0e2952f8726e7484ecb42a2202c0825b232b4c4c Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Wed, 7 Sep 2022 22:47:20 +0100 Subject: [PATCH 06/14] Made similar code even more similar. --- src/Math-Matrix/PMQRDecomposition.class.st | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index b443ec9d..ea4eea86 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -47,9 +47,12 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections * (householderVector at: 2) * matrixOfMinor). matrixOfMinor rowsWithIndexDo: [ :aRow :index | aRow withIndexDo: [ :element :column | + | rowNumber columnNumber | + rowNumber := col + index - 1. + columnNumber := col + column - 1. r - rowAt: col + index - 1 - columnAt: col + column - 1 + rowAt: rowNumber + columnAt: columnNumber put: ((element closeTo: 0) ifTrue: [ 0 ] ifFalse: [ element ]) ] ] ]. @@ -95,9 +98,12 @@ PMQRDecomposition >> decomposeWithPivot [ * (householderVector at: 2) * matrixOfMinor). matrixOfMinor rowsWithIndexDo: [ :aRow :index | aRow withIndexDo: [ :element :column | + | rowNumber columnNumber | + rowNumber := rank + index - 1. + columnNumber := rank + column - 1. r - rowAt: rank + index - 1 - columnAt: rank + column - 1 + rowAt: rowNumber + columnAt: columnNumber put: ((element closeTo: 0) ifTrue: [ 0 ] ifFalse: [ element ]) ] ]. From bc7c95d234e46899baac18b542aca06c946041ce Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 11 Sep 2022 13:30:35 +0100 Subject: [PATCH 07/14] Moved the declaration closer to the site of the computation. --- src/Math-Matrix/PMQRDecomposition.class.st | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index ea4eea86..d8cd6110 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -29,13 +29,13 @@ that describes the mechanics: https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections " - | identityMatrix householderVector i matrixOfMinor | - identityMatrix := PMSymmetricMatrix identity: colSize. + | i matrixOfMinor | 1 to: self numberOfColumns do: [ :col | - | columnVectorFromRMatrix householderMatrix v | + | householderVector householderMatrix columnVectorFromRMatrix identityMatrix v | columnVectorFromRMatrix := r columnVectorAt: col size: colSize. householderVector := columnVectorFromRMatrix householder. v := (PMVector zeros: col - 1) , (householderVector at: 2). + identityMatrix := PMSymmetricMatrix identity: colSize. householderMatrix := identityMatrix - ((householderVector at: 1) * v tensorProduct: v). From 1bd05784e043f3a164b444f09f527f1cc3348cac Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 11 Sep 2022 13:39:55 +0100 Subject: [PATCH 08/14] Extracted a key computation step to an intention-revealing method. --- src/Math-Matrix/PMQRDecomposition.class.st | 29 ++++++++++++++++------ 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index d8cd6110..b2aeced3 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -29,16 +29,16 @@ that describes the mechanics: https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections " - | i matrixOfMinor | + | i matrixOfMinor householderReflection | 1 to: self numberOfColumns do: [ :col | - | householderVector householderMatrix columnVectorFromRMatrix identityMatrix v | + | householderVector householderMatrix columnVectorFromRMatrix | columnVectorFromRMatrix := r columnVectorAt: col size: colSize. - householderVector := columnVectorFromRMatrix householder. - v := (PMVector zeros: col - 1) , (householderVector at: 2). - identityMatrix := PMSymmetricMatrix identity: colSize. - householderMatrix := identityMatrix - - - ((householderVector at: 1) * v tensorProduct: v). + householderReflection := self + householderReflectionOf: + columnVectorFromRMatrix + atColumnNumber: col. + householderVector := householderReflection at: 1. + householderMatrix := householderReflection at: 2. q := q * householderMatrix. matrixOfMinor := r minor: col - 1 and: col - 1. matrixOfMinor := matrixOfMinor @@ -138,6 +138,19 @@ PMQRDecomposition >> decomposeWithPivot [ ^ Array with: q with: r with: pivot ] +{ #category : #private } +PMQRDecomposition >> householderReflectionOf: columnVector atColumnNumber: columnNumber [ + + | householderVector v identityMatrix householderMatrix | + householderVector := columnVector householder. + v := (PMVector zeros: columnNumber - 1) , (householderVector at: 2). + identityMatrix := PMSymmetricMatrix identity: colSize. + householderMatrix := identityMatrix + - + ((householderVector at: 1) * v tensorProduct: v). + ^ Array with: householderVector with: householderMatrix . +] + { #category : #private } PMQRDecomposition >> initialQMatrix [ From b27061309192c7b4a95fa5e92045cbcafb3bbf22 Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 11 Sep 2022 13:51:33 +0100 Subject: [PATCH 09/14] Made similar code even more similar. --- src/Math-Matrix/PMQRDecomposition.class.st | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index b2aeced3..b0ab6118 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -79,13 +79,13 @@ PMQRDecomposition >> decomposeWithPivot [ rank := 0. identityMatrix := PMSymmetricMatrix identity: colSize. [ - | householderMatrix | + | householderMatrix columnVectorFromRMatrix | rank := rank + 1. pivot at: rank put: mx. r swapColumn: rank withColumn: mx. vectorOfNormSquareds swap: rank with: mx. - householderVector := (r columnVectorAt: rank size: colSize) - householder. + columnVectorFromRMatrix := r columnVectorAt: rank size: colSize. + householderVector := columnVectorFromRMatrix householder. v := (PMVector zeros: rank - 1) , (householderVector at: 2). householderMatrix := identityMatrix - From 5fb1e1bb577ea7130fe1b518a0c5b444893a9c52 Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 11 Sep 2022 13:53:21 +0100 Subject: [PATCH 10/14] Reduced the scope of the identity matrix to the code block. --- src/Math-Matrix/PMQRDecomposition.class.st | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index b0ab6118..ec18ff20 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -71,15 +71,14 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections { #category : #arithmetic } PMQRDecomposition >> decomposeWithPivot [ - | identityMatrix householderVector i v vectorOfNormSquareds rank mx pivot matrixOfMinor | + | householderVector i v vectorOfNormSquareds rank mx pivot matrixOfMinor | vectorOfNormSquareds := matrixToDecompose columnsCollect: [ :columnVector | columnVector * columnVector ]. mx := vectorOfNormSquareds indexOf: vectorOfNormSquareds max. pivot := Array new: vectorOfNormSquareds size. rank := 0. - identityMatrix := PMSymmetricMatrix identity: colSize. [ - | householderMatrix columnVectorFromRMatrix | + | householderMatrix columnVectorFromRMatrix identityMatrix | rank := rank + 1. pivot at: rank put: mx. r swapColumn: rank withColumn: mx. @@ -87,6 +86,7 @@ PMQRDecomposition >> decomposeWithPivot [ columnVectorFromRMatrix := r columnVectorAt: rank size: colSize. householderVector := columnVectorFromRMatrix householder. v := (PMVector zeros: rank - 1) , (householderVector at: 2). + identityMatrix := PMSymmetricMatrix identity: colSize. householderMatrix := identityMatrix - ((householderVector at: 1) * v tensorProduct: v). From 9baa7baafb2af6462d47b6fc22d763eeff24def4 Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 11 Sep 2022 13:58:54 +0100 Subject: [PATCH 11/14] We now use the new method and have reduced the scope of a temporary variable. --- src/Math-Matrix/PMQRDecomposition.class.st | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index ec18ff20..13281664 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -71,25 +71,25 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections { #category : #arithmetic } PMQRDecomposition >> decomposeWithPivot [ - | householderVector i v vectorOfNormSquareds rank mx pivot matrixOfMinor | + | i v vectorOfNormSquareds rank mx pivot matrixOfMinor | vectorOfNormSquareds := matrixToDecompose columnsCollect: [ :columnVector | columnVector * columnVector ]. mx := vectorOfNormSquareds indexOf: vectorOfNormSquareds max. pivot := Array new: vectorOfNormSquareds size. rank := 0. [ - | householderMatrix columnVectorFromRMatrix identityMatrix | + | householderReflection householderMatrix householderVector columnVectorFromRMatrix | rank := rank + 1. pivot at: rank put: mx. r swapColumn: rank withColumn: mx. vectorOfNormSquareds swap: rank with: mx. columnVectorFromRMatrix := r columnVectorAt: rank size: colSize. - householderVector := columnVectorFromRMatrix householder. - v := (PMVector zeros: rank - 1) , (householderVector at: 2). - identityMatrix := PMSymmetricMatrix identity: colSize. - householderMatrix := identityMatrix - - - ((householderVector at: 1) * v tensorProduct: v). + householderReflection := self + householderReflectionOf: + columnVectorFromRMatrix + atColumnNumber: rank. + householderVector := householderReflection at: 1. + householderMatrix := householderReflection at: 2. q := q * householderMatrix. matrixOfMinor := r minor: rank - 1 and: rank - 1. matrixOfMinor := matrixOfMinor From c34d9cce651e26a7fae728984c9c0d10a4701112 Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 11 Sep 2022 14:04:02 +0100 Subject: [PATCH 12/14] Removed an unused local variable. --- src/Math-Matrix/PMQRDecomposition.class.st | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index 13281664..e2da73d2 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -71,7 +71,7 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections { #category : #arithmetic } PMQRDecomposition >> decomposeWithPivot [ - | i v vectorOfNormSquareds rank mx pivot matrixOfMinor | + | i vectorOfNormSquareds rank mx pivot matrixOfMinor | vectorOfNormSquareds := matrixToDecompose columnsCollect: [ :columnVector | columnVector * columnVector ]. mx := vectorOfNormSquareds indexOf: vectorOfNormSquareds max. From 009e6778e2b37b375df8cb433d0269946d219f82 Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 11 Sep 2022 14:42:30 +0100 Subject: [PATCH 13/14] Reduced the scope of a temporary variable and made similar code (variable listing) even more similar. --- src/Math-Matrix/PMQRDecomposition.class.st | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index e2da73d2..237c4c3c 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -29,9 +29,9 @@ that describes the mechanics: https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections " - | i matrixOfMinor householderReflection | + | i matrixOfMinor | 1 to: self numberOfColumns do: [ :col | - | householderVector householderMatrix columnVectorFromRMatrix | + | householderReflection householderMatrix householderVector columnVectorFromRMatrix | columnVectorFromRMatrix := r columnVectorAt: col size: colSize. householderReflection := self householderReflectionOf: From 04cc08a5cf8816b6376b23acbf5fe55a2ea80bff Mon Sep 17 00:00:00 2001 From: hemalvarambhia Date: Sun, 25 Sep 2022 10:19:34 +0100 Subject: [PATCH 14/14] Improved the name of a local variable. --- src/Math-Matrix/PMQRDecomposition.class.st | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/Math-Matrix/PMQRDecomposition.class.st b/src/Math-Matrix/PMQRDecomposition.class.st index 237c4c3c..f5213976 100644 --- a/src/Math-Matrix/PMQRDecomposition.class.st +++ b/src/Math-Matrix/PMQRDecomposition.class.st @@ -71,18 +71,18 @@ https://en.wikipedia.org/wiki/QR_decomposition#Using_Householder_reflections { #category : #arithmetic } PMQRDecomposition >> decomposeWithPivot [ - | i vectorOfNormSquareds rank mx pivot matrixOfMinor | + | i vectorOfNormSquareds rank positionOfMaximum pivot matrixOfMinor | vectorOfNormSquareds := matrixToDecompose columnsCollect: [ :columnVector | columnVector * columnVector ]. - mx := vectorOfNormSquareds indexOf: vectorOfNormSquareds max. + positionOfMaximum := vectorOfNormSquareds indexOf: vectorOfNormSquareds max. pivot := Array new: vectorOfNormSquareds size. rank := 0. [ | householderReflection householderMatrix householderVector columnVectorFromRMatrix | rank := rank + 1. - pivot at: rank put: mx. - r swapColumn: rank withColumn: mx. - vectorOfNormSquareds swap: rank with: mx. + pivot at: rank put: positionOfMaximum. + r swapColumn: rank withColumn: positionOfMaximum. + vectorOfNormSquareds swap: rank with: positionOfMaximum. columnVectorFromRMatrix := r columnVectorAt: rank size: colSize. householderReflection := self householderReflectionOf: @@ -115,16 +115,16 @@ PMQRDecomposition >> decomposeWithPivot [ - (r rowAt: rank columnAt: ind) squared ]. rank < vectorOfNormSquareds size ifTrue: [ - mx := (vectorOfNormSquareds + positionOfMaximum := (vectorOfNormSquareds copyFrom: rank + 1 to: vectorOfNormSquareds size) max. - (mx closeTo: 0) ifTrue: [ mx := 0 ]. - mx := mx > 0 + (positionOfMaximum closeTo: 0) ifTrue: [ positionOfMaximum := 0 ]. + positionOfMaximum := positionOfMaximum > 0 ifTrue: [ - vectorOfNormSquareds indexOf: mx startingAt: rank + 1 ] + vectorOfNormSquareds indexOf: positionOfMaximum startingAt: rank + 1 ] ifFalse: [ 0 ] ] - ifFalse: [ mx := 0 ]. - mx > 0 ] whileTrue. + ifFalse: [ positionOfMaximum := 0 ]. + positionOfMaximum > 0 ] whileTrue. i := 0. [ (r rowAt: colSize) isZero ] whileTrue: [ i := i + 1.