From 2c13efcfe028392c9d9759d8ffbf5de396e872cd Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 13:08:22 -0700 Subject: [PATCH 01/25] First draft of the test case for transform regression fix (5493) --- .ci/atime/tests.R | 56 ++++++++++------------------------------------- 1 file changed, 11 insertions(+), 45 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index fddfb1347b..0b36e3917c 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -73,52 +73,18 @@ test.list <- atime::atime_test_list( paste0('useDynLib(', new.Package_)) }, - # Performance regression discussed in: https://github.com/Rdatatable/data.table/issues/4311 - # Fixed in: https://github.com/Rdatatable/data.table/pull/4440 - "shallow regression fixed in #4440" = atime::atime_test( - N = 10^seq(3,8), + # Fixed in: https://github.com/Rdatatable/data.table/pull/5493 (off-branch) + # Merged to master in: https://github.com/Rdatatable/data.table/commit/2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7 + "transform regression fixed in #5493" = atime::atime_test( + N=10^seq(1, 20), setup = { - set.seed(1L) - dt <- data.table(a = sample.int(N)) - setindexv(dt, "a") + set.seed(108) + df <- data.frame(x = runif(N)) + dt <- as.data.table(df) }, - expr = data.table:::shallow(dt), - # Before = "", This needs to be updated later as there are two issues here: A) The source of regression (or the particular commit that led to it) is not clear; B) Older versions of data.table are having problems when being installed in this manner (This includes commits from before March 20 2020, when the issue that discovered or first mentioned the regression was created) - Regression = "b1b1832b0d2d4032b46477d9fe6efb15006664f4", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/0f0e7127b880df8459b0ed064dc841acd22f5b73) in the PR (https://github.com/Rdatatable/data.table/pull/4440/commits) that fixes the regression - Fixed = "9d3b9202fddb980345025a4f6ac451ed26a423be"), # Merge commit in the PR that fixed the regression (https://github.com/Rdatatable/data.table/pull/4440) - - # Test based on: https://github.com/Rdatatable/data.table/issues/5424 - # Performance regression introduced from a commit in: https://github.com/Rdatatable/data.table/pull/4491 - # Fixed in: https://github.com/Rdatatable/data.table/pull/5463 - "memrecycle regression fixed in #5463" = atime::atime_test( - N = 10^seq(3, 8), - setup = { - n <- N/100 - set.seed(2L) - dt <- data.table( - g = sample(seq_len(n), N, TRUE), - x = runif(N), - key = "g") - dt_mod <- copy(dt) - }, - expr = data.table:::`[.data.table`(dt_mod, , N := .N, by = g), - Before = "be2f72e6f5c90622fe72e1c315ca05769a9dc854", # Parent of the regression causing commit (https://github.com/Rdatatable/data.table/commit/e793f53466d99f86e70fc2611b708ae8c601a451) in the PR that introduced the issue (https://github.com/Rdatatable/data.table/pull/4491/commits) - Regression = "e793f53466d99f86e70fc2611b708ae8c601a451", # Commit responsible for regression in the PR that introduced the issue (https://github.com/Rdatatable/data.table/pull/4491/commits) - Fixed = "58409197426ced4714af842650b0cc3b9e2cb842"), # Last commit in the PR that fixed the regression (https://github.com/Rdatatable/data.table/pull/5463/commits) - - # Issue reported in: https://github.com/Rdatatable/data.table/issues/5426 - # To be fixed in: https://github.com/Rdatatable/data.table/pull/5427 - "setDT improved in #5427" = atime::atime_test( - N = 10^seq(1, 7), - setup = { - L <- replicate(N, 1, simplify = FALSE) - setDT(L) - }, - expr = { - data.table:::setattr(L, "class", NULL) - data.table:::setDT(L) - }, - Slow = "c4a2085e35689a108d67dacb2f8261e4964d7e12", # Parent of the first commit in the PR that fixes the issue (https://github.com/Rdatatable/data.table/commit/7cc4da4c1c8e568f655ab5167922dcdb75953801) - Fast = "1872f473b20fdcddc5c1b35d79fe9229cd9a1d15") # Last commit in the PR that fixes the issue (https://github.com/Rdatatable/data.table/pull/5427/commits) + expr = data.table:::`[.data.table`(transform(dt, y = round(x))), + Before = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", # Parent of the commit that fixed the regression. + # Regression = "", + Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) # nolint end: undesirable_operator_linter. From 992608dfcb1badc4c6c2976b68c2838bb9927bac Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 15:44:05 -0700 Subject: [PATCH 02/25] Update tests.R --- .ci/atime/tests.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 0b36e3917c..c2712727c1 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -76,7 +76,7 @@ test.list <- atime::atime_test_list( # Fixed in: https://github.com/Rdatatable/data.table/pull/5493 (off-branch) # Merged to master in: https://github.com/Rdatatable/data.table/commit/2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7 "transform regression fixed in #5493" = atime::atime_test( - N=10^seq(1, 20), + N = 10^seq(1, 20), setup = { set.seed(108) df <- data.frame(x = runif(N)) @@ -85,6 +85,8 @@ test.list <- atime::atime_test_list( expr = data.table:::`[.data.table`(transform(dt, y = round(x))), Before = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", # Parent of the commit that fixed the regression. # Regression = "", - Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") + Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7", + After = "f505965752df1377c96636685d5e7690f8fa4cf7", + Jan24 = "dc4c1ea1a23022c467f68c029f04082d8992914d") ) # nolint end: undesirable_operator_linter. From 042e359935e00da83a9d4e7b8b9336511bf91f9d Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 19:50:43 -0700 Subject: [PATCH 03/25] Update tests.R --- .ci/atime/tests.R | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index c2712727c1..440ea16b36 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,10 +83,8 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), - Before = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", # Parent of the commit that fixed the regression. + Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", # Parent of the commit that fixed the regression. # Regression = "", - Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7", - After = "f505965752df1377c96636685d5e7690f8fa4cf7", - Jan24 = "dc4c1ea1a23022c467f68c029f04082d8992914d") + Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) # nolint end: undesirable_operator_linter. From 8bd7e0cea4e8e28926b7e3ace1185b8eb0f1a195 Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 20:16:20 -0700 Subject: [PATCH 04/25] Update tests.R --- .ci/atime/tests.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 440ea16b36..bd8cd04efa 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -85,6 +85,7 @@ test.list <- atime::atime_test_list( expr = data.table:::`[.data.table`(transform(dt, y = round(x))), Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", # Parent of the commit that fixed the regression. # Regression = "", - Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") + Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7", + Fixed = "f505965752df1377c96636685d5e7690f8fa4cf7") ) # nolint end: undesirable_operator_linter. From 30735c36739a290170b5053b366ae4bf6627a5cd Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 20:34:27 -0700 Subject: [PATCH 05/25] Update tests.R --- .ci/atime/tests.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index bd8cd04efa..32a208472a 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -85,7 +85,6 @@ test.list <- atime::atime_test_list( expr = data.table:::`[.data.table`(transform(dt, y = round(x))), Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", # Parent of the commit that fixed the regression. # Regression = "", - Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7", - Fixed = "f505965752df1377c96636685d5e7690f8fa4cf7") + Fast = "f505965752df1377c96636685d5e7690f8fa4cf7") ) # nolint end: undesirable_operator_linter. From 8e79a5d28e7de446e8788665f7dc325af26b5d9d Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 21:09:28 -0700 Subject: [PATCH 06/25] Update tests.R --- .ci/atime/tests.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 32a208472a..18e36d788c 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,8 +83,8 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), - Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", # Parent of the commit that fixed the regression. + Regression = "0895fa247afcf6b38044bd5f56c0d209691ddb31", # Regression = "", - Fast = "f505965752df1377c96636685d5e7690f8fa4cf7") + Fixed = "19a73c795630a5f8c1eccc735bf38f6e47e09e6a") ) # nolint end: undesirable_operator_linter. From 1d99007d2264cf94ce4654c9bb7b54b745a4e931 Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 21:58:42 -0700 Subject: [PATCH 07/25] Update tests.R --- .ci/atime/tests.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 18e36d788c..d7cf368ffb 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,8 +83,9 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), + Before = "0895fa247afcf6b38044bd5f56c0d209691ddb31", Regression = "0895fa247afcf6b38044bd5f56c0d209691ddb31", # Regression = "", - Fixed = "19a73c795630a5f8c1eccc735bf38f6e47e09e6a") + Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) # nolint end: undesirable_operator_linter. From 80362d59a71c4db46e4f0f2f15fd5e724ea69ae6 Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 22:19:58 -0700 Subject: [PATCH 08/25] weird results --- .ci/atime/tests.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index d7cf368ffb..0061d481fe 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,9 +83,8 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), - Before = "0895fa247afcf6b38044bd5f56c0d209691ddb31", + # Before = "", Regression = "0895fa247afcf6b38044bd5f56c0d209691ddb31", - # Regression = "", Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) # nolint end: undesirable_operator_linter. From 4b7cd7acc3b2ff3bf227fbf6580f6c2f9a6ae4c4 Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 22:38:01 -0700 Subject: [PATCH 09/25] Update tests.R --- .ci/atime/tests.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 0061d481fe..858a96e47b 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -78,12 +78,11 @@ test.list <- atime::atime_test_list( "transform regression fixed in #5493" = atime::atime_test( N = 10^seq(1, 20), setup = { - set.seed(108) df <- data.frame(x = runif(N)) dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), - # Before = "", + Before = "0895fa247afcf6b38044bd5f56c0d209691ddb31", Regression = "0895fa247afcf6b38044bd5f56c0d209691ddb31", Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) From c1b518a35ac5e79a133993c22fa04274645f6a09 Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 18 Jul 2024 23:13:10 -0700 Subject: [PATCH 10/25] Update tests.R --- .ci/atime/tests.R | 1 + 1 file changed, 1 insertion(+) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 858a96e47b..08266de1bb 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -78,6 +78,7 @@ test.list <- atime::atime_test_list( "transform regression fixed in #5493" = atime::atime_test( N = 10^seq(1, 20), setup = { + set.seed(108) df <- data.frame(x = runif(N)) dt <- as.data.table(df) }, From 1fd4e1259700a0cf80a622fe2dd9608df7e098a1 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 19 Jul 2024 11:12:24 -0700 Subject: [PATCH 11/25] Update tests.R --- .ci/atime/tests.R | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 08266de1bb..87ac44b744 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -75,16 +75,14 @@ test.list <- atime::atime_test_list( # Fixed in: https://github.com/Rdatatable/data.table/pull/5493 (off-branch) # Merged to master in: https://github.com/Rdatatable/data.table/commit/2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7 - "transform regression fixed in #5493" = atime::atime_test( + "transform improved in #5493" = atime::atime_test( N = 10^seq(1, 20), setup = { - set.seed(108) df <- data.frame(x = runif(N)) dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), - Before = "0895fa247afcf6b38044bd5f56c0d209691ddb31", - Regression = "0895fa247afcf6b38044bd5f56c0d209691ddb31", - Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") + Slow = "0895fa247afcf6b38044bd5f56c0d209691ddb31",, + Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) # nolint end: undesirable_operator_linter. From 62302d2788ed91926e3c41087735e5b3e48ae492 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 19 Jul 2024 12:01:20 -0700 Subject: [PATCH 12/25] Update tests.R --- .ci/atime/tests.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 87ac44b744..f614c0a304 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -82,7 +82,7 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), - Slow = "0895fa247afcf6b38044bd5f56c0d209691ddb31",, + Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) # nolint end: undesirable_operator_linter. From 43b731f2e042d6159111842f8d0320ab73d29fe0 Mon Sep 17 00:00:00 2001 From: Ani Date: Wed, 14 Aug 2024 11:15:36 -0700 Subject: [PATCH 13/25] Update tests.R --- .ci/atime/tests.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index f614c0a304..7d500956a4 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -82,7 +82,7 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), - Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", - Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") + Regression = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", + Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7") ) # nolint end: undesirable_operator_linter. From 3c571a83e628b7ad80b2355099546732ac38bbf8 Mon Sep 17 00:00:00 2001 From: Ani Date: Wed, 14 Aug 2024 15:15:02 -0700 Subject: [PATCH 14/25] Update tests.R --- .ci/atime/tests.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index b2b088ad7c..da3d5511d2 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -82,8 +82,9 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, y = round(x))), + Before = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", Regression = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7"), NULL) -# nolint end: undesirable_operator_linter. \ No newline at end of file +# nolint end: undesirable_operator_linter. From 55148c17a9a70c180eba0093a7a399815158e340 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 13 Sep 2024 12:29:12 -0700 Subject: [PATCH 15/25] More columns/ops --- .ci/atime/tests.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index da3d5511d2..f4c79731c0 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -78,10 +78,10 @@ test.list <- atime::atime_test_list( "transform improved in #5493" = atime::atime_test( N = 10^seq(1, 20), setup = { - df <- data.frame(x = runif(N)) + df <- data.frame(x = runif(N), y = rnorm(N), z = sample(letters, N, replace = TRUE)) dt <- as.data.table(df) }, - expr = data.table:::`[.data.table`(transform(dt, y = round(x))), + expr = data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z))), Before = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", Regression = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7"), From 86ea7579b4c9f2f4a08daaa665eda28dffbccb5a Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 13 Sep 2024 12:43:50 -0700 Subject: [PATCH 16/25] Updated SHAs --- .ci/atime/tests.R | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 89b2d31070..19a5cfcb12 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -84,13 +84,8 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z))), - Before = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", - Regression = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", - Fixed = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7"), - - }, - Slow = "c4a2085e35689a108d67dacb2f8261e4964d7e12", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/7cc4da4c1c8e568f655ab5167922dcdb75953801) in the PR (https://github.com/Rdatatable/data.table/pull/5427/commits) that fixes the issue - Fast = "af48a805e7a5026a0c2d0a7fd9b587fea5cfa3c4"), # Last commit in the PR (https://github.com/Rdatatable/data.table/pull/5427/commits) that fixes the issue + Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", + Fast = "cde7333938a590a3cddbda1b02103650e2f55d15"), NULL) # nolint end: undesirable_operator_linter. From 5d7c2147a7acee6b199c666836231ea463cefb9e Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 13 Sep 2024 12:56:29 -0700 Subject: [PATCH 17/25] Test without the hack --- .ci/atime/tests.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 19a5cfcb12..4943b406d1 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,7 +83,7 @@ test.list <- atime::atime_test_list( df <- data.frame(x = runif(N), y = rnorm(N), z = sample(letters, N, replace = TRUE)) dt <- as.data.table(df) }, - expr = data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z))), + expr = data.table:::transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z)), Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", Fast = "cde7333938a590a3cddbda1b02103650e2f55d15"), From 977c3bd7b49bfd7b002a7f0a925a8a4b8fd48ca0 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 13 Sep 2024 13:25:48 -0700 Subject: [PATCH 18/25] Adding a conditional op into the mix --- .ci/atime/tests.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 4943b406d1..2991482dd1 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,7 +83,7 @@ test.list <- atime::atime_test_list( df <- data.frame(x = runif(N), y = rnorm(N), z = sample(letters, N, replace = TRUE)) dt <- as.data.table(df) }, - expr = data.table:::transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z)), + expr = data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z), conditional = ifelse(y > 0, x * y, x / y))), Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", Fast = "cde7333938a590a3cddbda1b02103650e2f55d15"), From 28464678ffe4f25ba1eb91b3bbc1f48375f422f5 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 13 Sep 2024 13:43:28 -0700 Subject: [PATCH 19/25] Test modification addition of columns by reference (in case the commit somehow affects it) --- .ci/atime/tests.R | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 2991482dd1..5e8a8b1a27 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,7 +83,8 @@ test.list <- atime::atime_test_list( df <- data.frame(x = runif(N), y = rnorm(N), z = sample(letters, N, replace = TRUE)) dt <- as.data.table(df) }, - expr = data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z), conditional = ifelse(y > 0, x * y, x / y))), + expr = data.table:::`[.data.table`(dt[, `:=`(xRounded = round(x), ySquared = y^2, zLength = nchar(z))]), + # data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z), conditional = ifelse(y > 0, x * y, x / y))), Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", Fast = "cde7333938a590a3cddbda1b02103650e2f55d15"), From 0e394c887c8300f79f5ac0b25491cb29e8a2f72b Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 13 Sep 2024 13:55:20 -0700 Subject: [PATCH 20/25] Testing all the SHAs in master that were referenced in the PR thread. --- .ci/atime/tests.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 5e8a8b1a27..6188b8b993 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -86,7 +86,10 @@ test.list <- atime::atime_test_list( expr = data.table:::`[.data.table`(dt[, `:=`(xRounded = round(x), ySquared = y^2, zLength = nchar(z))]), # data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z), conditional = ifelse(y > 0, x * y, x / y))), Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", - Fast = "cde7333938a590a3cddbda1b02103650e2f55d15"), + Fast = "cde7333938a590a3cddbda1b02103650e2f55d15", + Next = "18a7209b242fa2a784fe76be845106baacd8349f", + Next2 = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7", + Next3 = "6db0eda711bb59ad9b6009208584c56da1abb915"), NULL) # nolint end: undesirable_operator_linter. From 31eb7651ed4f5649ab0f5a93c1b313fcd5c3355a Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 13 Sep 2024 14:04:58 -0700 Subject: [PATCH 21/25] Update tests.R --- .ci/atime/tests.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 6188b8b993..b02d0687c9 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -83,8 +83,7 @@ test.list <- atime::atime_test_list( df <- data.frame(x = runif(N), y = rnorm(N), z = sample(letters, N, replace = TRUE)) dt <- as.data.table(df) }, - expr = data.table:::`[.data.table`(dt[, `:=`(xRounded = round(x), ySquared = y^2, zLength = nchar(z))]), - # data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z), conditional = ifelse(y > 0, x * y, x / y))), + expr = data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z), conditional = ifelse(y > 0, x * y, x / y))), Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", Fast = "cde7333938a590a3cddbda1b02103650e2f55d15", Next = "18a7209b242fa2a784fe76be845106baacd8349f", From 936f26fc063f4e135f04436847a36087672af1b1 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 20 Sep 2024 10:59:45 -0700 Subject: [PATCH 22/25] Using transform.data.table, simplified test case back to OP's example --- .ci/atime/tests.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index b02d0687c9..cd9f5e9eea 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -80,10 +80,10 @@ test.list <- atime::atime_test_list( "transform improved in #5493" = atime::atime_test( N = 10^seq(1, 20), setup = { - df <- data.frame(x = runif(N), y = rnorm(N), z = sample(letters, N, replace = TRUE)) + df <- data.frame(x = runif(N)) dt <- as.data.table(df) }, - expr = data.table:::`[.data.table`(transform(dt, xRounded = round(x), ySquared = y^2, zLength = nchar(z), conditional = ifelse(y > 0, x * y, x / y))), + expr = data.table:::transform.data.table(dt, y = round(x)), Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", Fast = "cde7333938a590a3cddbda1b02103650e2f55d15", Next = "18a7209b242fa2a784fe76be845106baacd8349f", From e1e5c4da350c276f8969c5e73ec733013c2350b9 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 20 Sep 2024 11:09:11 -0700 Subject: [PATCH 23/25] Correct SHAs --- .ci/atime/tests.R | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index cd9f5e9eea..3641807e21 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -84,11 +84,8 @@ test.list <- atime::atime_test_list( dt <- as.data.table(df) }, expr = data.table:::transform.data.table(dt, y = round(x)), - Slow = "64e2041ad48838679ac4c901a04557f5a7ba2df3", - Fast = "cde7333938a590a3cddbda1b02103650e2f55d15", - Next = "18a7209b242fa2a784fe76be845106baacd8349f", - Next2 = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7", - Next3 = "6db0eda711bb59ad9b6009208584c56da1abb915"), + Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", + Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7"), NULL) # nolint end: undesirable_operator_linter. From 0c3ed8242bef60bbbf5ed377ddee11e65c84735d Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 20 Sep 2024 13:23:33 -0700 Subject: [PATCH 24/25] seconds.limit++ --- .ci/atime/tests.R | 1 + 1 file changed, 1 insertion(+) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 3641807e21..290c3a6c78 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -79,6 +79,7 @@ test.list <- atime::atime_test_list( # Merged to master in: https://github.com/Rdatatable/data.table/commit/2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7 "transform improved in #5493" = atime::atime_test( N = 10^seq(1, 20), + seconds.limit = 0.1, setup = { df <- data.frame(x = runif(N)) dt <- as.data.table(df) From 7dcfe8436a901b5c54fb08d0d92eeaf195ea02a6 Mon Sep 17 00:00:00 2001 From: Ani Date: Fri, 20 Sep 2024 13:56:09 -0700 Subject: [PATCH 25/25] Test the change in SHA here too, and go with the default spec for seconds.limit --- .ci/atime/tests.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 290c3a6c78..08f845484a 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -79,13 +79,12 @@ test.list <- atime::atime_test_list( # Merged to master in: https://github.com/Rdatatable/data.table/commit/2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7 "transform improved in #5493" = atime::atime_test( N = 10^seq(1, 20), - seconds.limit = 0.1, setup = { df <- data.frame(x = runif(N)) dt <- as.data.table(df) }, expr = data.table:::transform.data.table(dt, y = round(x)), - Slow = "bf499090c0e6fd5cb492bf8b1603d93c1ee21dfb", + Slow = "0895fa247afcf6b38044bd5f56c0d209691ddb31", Fast = "2d1a0575f87cc50e90f64825c30d7a6cb6b05dd7"), NULL)