From 174f05c799d342ab05b642bc043380f4b897abdb Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 5 Sep 2024 12:46:52 -0700 Subject: [PATCH 1/9] Test changes made for consistency first. --- .ci/atime/tests.R | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 663e23b5c7..17ad7212c5 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -78,7 +78,7 @@ test.list <- atime::atime_test_list( # 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), + N = 10^seq(3, 8), setup = { set.seed(1L) dt <- data.table(a = sample.int(N)) @@ -129,14 +129,12 @@ test.list <- atime::atime_test_list( N = 10^seq(1, 20), setup = { d <- data.table( - id3 = sample(c(seq.int(N*0.9), sample( N*0.9, N*0.1, TRUE))), + id = sample(c(seq.int(N * 0.9), sample(N * 0.9, N * 0.1, TRUE))), v1 = sample(5L, N, TRUE), v2 = sample(5L, N, TRUE) ) }, - expr = { - expr=data.table:::`[.data.table`(d, , max(v1) - min(v2), by = id3) - }, + expr = data.table:::`[.data.table`(d, , max(v1) - min(v2), by = id), Before = "7a9eaf62ede487625200981018d8692be8c6f134", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/515de90a6068911a148e54343a3503043b8bb87c) in the PR (https://github.com/Rdatatable/data.table/pull/4164/commits) that introduced the regression Regression = "c152ced0e5799acee1589910c69c1a2c6586b95d", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/15f0598b9828d3af2eb8ddc9b38e0356f42afe4f) in the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression Fixed = "f750448a2efcd258b3aba57136ee6a95ce56b302"), # Second commit of the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression @@ -151,9 +149,7 @@ test.list <- atime::atime_test_list( setkey(L, V1) }, ## New DT can safely retain key. - expr = { - data.table:::`[.data.table`(L, , .SD) - }, + expr = data.table:::`[.data.table`(L, , .SD), Fast = "353dc7a6b66563b61e44b2fa0d7b73a0f97ca461", # Close-to-last merge commit in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slow = "3ca83738d70d5597d9e168077f3768e32569c790", # Circa 2024 master parent of close-to-last merge commit (https://github.com/Rdatatable/data.table/commit/353dc7a6b66563b61e44b2fa0d7b73a0f97ca461) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slower = "cacdc92df71b777369a217b6c902c687cf35a70d"), # Circa 2020 parent of the first commit (https://github.com/Rdatatable/data.table/commit/74636333d7da965a11dad04c322c752a409db098) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue From c6f63c0f94ca336c0d207b7a3f73b470faacceec Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 5 Sep 2024 13:15:10 -0700 Subject: [PATCH 2/9] 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 17ad7212c5..8a18818e5d 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -149,7 +149,7 @@ test.list <- atime::atime_test_list( setkey(L, V1) }, ## New DT can safely retain key. - expr = data.table:::`[.data.table`(L, , .SD), + expr = { data.table:::`[.data.table`(L, , .SD) }, Fast = "353dc7a6b66563b61e44b2fa0d7b73a0f97ca461", # Close-to-last merge commit in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slow = "3ca83738d70d5597d9e168077f3768e32569c790", # Circa 2024 master parent of close-to-last merge commit (https://github.com/Rdatatable/data.table/commit/353dc7a6b66563b61e44b2fa0d7b73a0f97ca461) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slower = "cacdc92df71b777369a217b6c902c687cf35a70d"), # Circa 2020 parent of the first commit (https://github.com/Rdatatable/data.table/commit/74636333d7da965a11dad04c322c752a409db098) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue From 542462ed0ea3fca4b49c8404a5c1bd2744f0480e Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 5 Sep 2024 13:57:18 -0700 Subject: [PATCH 3/9] 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 8a18818e5d..17ad7212c5 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -149,7 +149,7 @@ test.list <- atime::atime_test_list( setkey(L, V1) }, ## New DT can safely retain key. - expr = { data.table:::`[.data.table`(L, , .SD) }, + expr = data.table:::`[.data.table`(L, , .SD), Fast = "353dc7a6b66563b61e44b2fa0d7b73a0f97ca461", # Close-to-last merge commit in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slow = "3ca83738d70d5597d9e168077f3768e32569c790", # Circa 2024 master parent of close-to-last merge commit (https://github.com/Rdatatable/data.table/commit/353dc7a6b66563b61e44b2fa0d7b73a0f97ca461) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slower = "cacdc92df71b777369a217b6c902c687cf35a70d"), # Circa 2020 parent of the first commit (https://github.com/Rdatatable/data.table/commit/74636333d7da965a11dad04c322c752a409db098) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue From 3c942178adbf0c18b6319b5463a1647547c3481b Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 5 Sep 2024 18:04:09 -0700 Subject: [PATCH 4/9] 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 17ad7212c5..803c224961 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -129,12 +129,14 @@ test.list <- atime::atime_test_list( N = 10^seq(1, 20), setup = { d <- data.table( - id = sample(c(seq.int(N * 0.9), sample(N * 0.9, N * 0.1, TRUE))), + id3 = sample(c(seq.int(N*0.9), sample( N*0.9, N*0.1, TRUE))), v1 = sample(5L, N, TRUE), v2 = sample(5L, N, TRUE) ) }, - expr = data.table:::`[.data.table`(d, , max(v1) - min(v2), by = id), + expr = { + expr=data.table:::`[.data.table`(d, , max(v1) - min(v2), by = id3) + }, Before = "7a9eaf62ede487625200981018d8692be8c6f134", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/515de90a6068911a148e54343a3503043b8bb87c) in the PR (https://github.com/Rdatatable/data.table/pull/4164/commits) that introduced the regression Regression = "c152ced0e5799acee1589910c69c1a2c6586b95d", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/15f0598b9828d3af2eb8ddc9b38e0356f42afe4f) in the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression Fixed = "f750448a2efcd258b3aba57136ee6a95ce56b302"), # Second commit of the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression From c2327d2dffba70f599b41475190a551c006c137d Mon Sep 17 00:00:00 2001 From: Ani Date: Thu, 5 Sep 2024 18:51:45 -0700 Subject: [PATCH 5/9] 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 803c224961..c0ab75bdfe 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -129,14 +129,12 @@ test.list <- atime::atime_test_list( N = 10^seq(1, 20), setup = { d <- data.table( - id3 = sample(c(seq.int(N*0.9), sample( N*0.9, N*0.1, TRUE))), + id = sample(c(seq.int(N*0.9), sample( N*0.9, N*0.1, TRUE))), v1 = sample(5L, N, TRUE), v2 = sample(5L, N, TRUE) ) }, - expr = { - expr=data.table:::`[.data.table`(d, , max(v1) - min(v2), by = id3) - }, + expr = data.table:::`[.data.table`(d, , max(v1) - min(v2), by = id), Before = "7a9eaf62ede487625200981018d8692be8c6f134", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/515de90a6068911a148e54343a3503043b8bb87c) in the PR (https://github.com/Rdatatable/data.table/pull/4164/commits) that introduced the regression Regression = "c152ced0e5799acee1589910c69c1a2c6586b95d", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/15f0598b9828d3af2eb8ddc9b38e0356f42afe4f) in the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression Fixed = "f750448a2efcd258b3aba57136ee6a95ce56b302"), # Second commit of the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression @@ -152,7 +150,7 @@ test.list <- atime::atime_test_list( }, ## New DT can safely retain key. expr = data.table:::`[.data.table`(L, , .SD), - Fast = "353dc7a6b66563b61e44b2fa0d7b73a0f97ca461", # Close-to-last merge commit in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue + Fast = "b594be79fcfd152a4bffb436dac95b2529e474ab", # Close-to-last merge commit in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slow = "3ca83738d70d5597d9e168077f3768e32569c790", # Circa 2024 master parent of close-to-last merge commit (https://github.com/Rdatatable/data.table/commit/353dc7a6b66563b61e44b2fa0d7b73a0f97ca461) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue Slower = "cacdc92df71b777369a217b6c902c687cf35a70d"), # Circa 2020 parent of the first commit (https://github.com/Rdatatable/data.table/commit/74636333d7da965a11dad04c322c752a409db098) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue From 8551f4f9367787cc9e5347635262ea961c17305b Mon Sep 17 00:00:00 2001 From: Ani Date: Mon, 9 Sep 2024 17:19:07 -0700 Subject: [PATCH 6/9] 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 c0ab75bdfe..7c27e3d641 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -165,6 +165,6 @@ test.list <- atime::atime_test_list( expr = data.table:::`[.data.table`(dt_mod, , 1, by = a, verbose = TRUE), Slow = "a01f00f7438daf4612280d6886e6929fa8c8f76e", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/fc0c1e76408c34a8482f16f7421d262c7f1bde32) in the PR (https://github.com/Rdatatable/data.table/pull/6296/commits) that fixes the issue Fast = "f248bbe6d1204dfc8def62328788eaadcc8e17a1"), # Merge commit of the PR (https://github.com/Rdatatable/data.table/pull/6296) that fixes the issue - + # Test. NULL) # nolint end: undesirable_operator_linter. From 904ad9362baa8c36723f58795a9192781c8e1cdf Mon Sep 17 00:00:00 2001 From: Ani Date: Mon, 9 Sep 2024 17:37:58 -0700 Subject: [PATCH 7/9] rm buggy SHA --- .ci/atime/tests.R | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/.ci/atime/tests.R b/.ci/atime/tests.R index 7c27e3d641..eaf19bc39c 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -139,21 +139,6 @@ test.list <- atime::atime_test_list( Regression = "c152ced0e5799acee1589910c69c1a2c6586b95d", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/15f0598b9828d3af2eb8ddc9b38e0356f42afe4f) in the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression Fixed = "f750448a2efcd258b3aba57136ee6a95ce56b302"), # Second commit of the PR (https://github.com/Rdatatable/data.table/pull/4558/commits) that fixes the regression - # Issue with sorting again when already sorted: https://github.com/Rdatatable/data.table/issues/4498 - # Fixed in: https://github.com/Rdatatable/data.table/pull/4501 - "DT[,.SD] improved in #4501" = atime::atime_test( - N = 10^seq(1, 10, by=0.5), - setup = { - set.seed(1) - L = as.data.table(as.character(rnorm(N, 1, 0.5))) - setkey(L, V1) - }, - ## New DT can safely retain key. - expr = data.table:::`[.data.table`(L, , .SD), - Fast = "b594be79fcfd152a4bffb436dac95b2529e474ab", # Close-to-last merge commit in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue - Slow = "3ca83738d70d5597d9e168077f3768e32569c790", # Circa 2024 master parent of close-to-last merge commit (https://github.com/Rdatatable/data.table/commit/353dc7a6b66563b61e44b2fa0d7b73a0f97ca461) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue - Slower = "cacdc92df71b777369a217b6c902c687cf35a70d"), # Circa 2020 parent of the first commit (https://github.com/Rdatatable/data.table/commit/74636333d7da965a11dad04c322c752a409db098) in the PR (https://github.com/Rdatatable/data.table/pull/4501/commits) that fixes the issue - # Issue reported in: https://github.com/Rdatatable/data.table/issues/6286 # Fixed in: https://github.com/Rdatatable/data.table/pull/6296 "DT[by, verbose = TRUE] improved in #6296" = atime::atime_test( From 3a13c59dfe6837f9d67aa4dedba52bba7e3df881 Mon Sep 17 00:00:00 2001 From: Ani Date: Mon, 9 Sep 2024 17:59:49 -0700 Subject: [PATCH 8/9] Test this with 1.2.1 --- .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 eaf19bc39c..340ab0119c 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -150,6 +150,6 @@ test.list <- atime::atime_test_list( expr = data.table:::`[.data.table`(dt_mod, , 1, by = a, verbose = TRUE), Slow = "a01f00f7438daf4612280d6886e6929fa8c8f76e", # Parent of the first commit (https://github.com/Rdatatable/data.table/commit/fc0c1e76408c34a8482f16f7421d262c7f1bde32) in the PR (https://github.com/Rdatatable/data.table/pull/6296/commits) that fixes the issue Fast = "f248bbe6d1204dfc8def62328788eaadcc8e17a1"), # Merge commit of the PR (https://github.com/Rdatatable/data.table/pull/6296) that fixes the issue - # Test. + NULL) # nolint end: undesirable_operator_linter. From aeb9f814fb8025b8d90b9351843acbfbe56daaa0 Mon Sep 17 00:00:00 2001 From: Ani Date: Mon, 9 Sep 2024 19:10:11 -0700 Subject: [PATCH 9/9] Fixed comments --- .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 340ab0119c..3cd1cc8457 100644 --- a/.ci/atime/tests.R +++ b/.ci/atime/tests.R @@ -120,8 +120,8 @@ test.list <- atime::atime_test_list( 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) + 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 = "1872f473b20fdcddc5c1b35d79fe9229cd9a1d15"), # Antepenultimate commit in the PR (https://github.com/Rdatatable/data.table/pull/5427/commits) that fixes the issue # Issue reported in: https://github.com/Rdatatable/data.table/issues/4200 # To be fixed in: https://github.com/Rdatatable/data.table/pull/4558