Skip to content

Commit 052b0fd

Browse files
CopilotQuafadas
andcommitted
Remove ineffective group lookup optimization from code review feedback
Code review correctly identified that caching group values in the while condition doesn't reduce array accesses since the array is still accessed on every iteration. Co-authored-by: Quafadas <24899792+Quafadas@users.noreply.github.com>
1 parent f9592a9 commit 052b0fd

3 files changed

Lines changed: 6 additions & 14 deletions

File tree

vecxtensions/src/Tower.scala

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,8 +429,7 @@ case class Tower( layers: Seq[Layer], id: UUID = UUID.randomUUID(), name: Optio
429429
var cumSum = 0.0
430430

431431
// Process block of same group, computing cumulative sum in-place
432-
var groupValue = g
433-
while i < numLosses && {groupValue = years(i); groupValue == g} do
432+
while i < numLosses && years(i) == g do
434433
cumSum += cededRaw(columnOffset + i)
435434
cededRaw(columnOffset + i) = cumSum
436435
i += 1
@@ -523,8 +522,7 @@ case class Tower( layers: Seq[Layer], id: UUID = UUID.randomUUID(), name: Optio
523522
var isFirstInGroup = true
524523

525524
// Process block of same group, computing differences in-place
526-
var groupValue = g
527-
while i < numLosses && {groupValue = years(i); groupValue == g} do
525+
while i < numLosses && years(i) == g do
528526
val currentValue = cededRaw(columnOffset + i)
529527
if isFirstInGroup then
530528
// First element in group gets its own value - no change needed

vecxtensions/src/groupCumSum.scala

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,7 @@ inline def groupCumSum(groups: Array[Int], values: Array[Double]): Array[Double]
1919
var cumSum = 0.0
2020

2121
// Process block of same group, computing cumulative sum
22-
// Optimize by caching group lookup
23-
var groupValue = g
24-
while i < n && {groupValue = groups(i); groupValue == g} do
22+
while i < n && groups(i) == g do
2523
cumSum += values(i)
2624
result(i) = cumSum
2725
i += 1
@@ -44,8 +42,7 @@ inline def groupCumSumInPlace(groups: Array[Int], values: Array[Double]): Unit =
4442
var cumSum = 0.0
4543

4644
// Process block of same group, computing cumulative sum in-place
47-
var groupValue = g
48-
while i < n && {groupValue = groups(i); groupValue == g} do
45+
while i < n && groups(i) == g do
4946
cumSum += values(i)
5047
values(i) = cumSum
5148
i += 1

vecxtensions/src/groupDiff.scala

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@ inline def groupDiff(groups: Array[Int], values: Array[Double]): Array[Double] =
2121
var isFirstInGroup = true
2222

2323
// Process block of same group, computing differences
24-
// Optimize by caching group lookup
25-
var groupValue = g
26-
while i < n && {groupValue = groups(i); groupValue == g} do
24+
while i < n && groups(i) == g do
2725
if isFirstInGroup then
2826
result(i) = values(i) // First element in group gets its own value
2927
isFirstInGroup = false
@@ -51,8 +49,7 @@ inline def groupDiffInPlace(groups: Array[Int], values: Array[Double]): Unit =
5149
var isFirstInGroup = true
5250

5351
// Process block of same group, computing differences in-place
54-
var groupValue = g
55-
while i < n && {groupValue = groups(i); groupValue == g} do
52+
while i < n && groups(i) == g do
5653
val currentValue = values(i)
5754
if isFirstInGroup then
5855
// First element in group gets its own value - no change needed

0 commit comments

Comments
 (0)