Skip to content
This repository was archived by the owner on Jun 20, 2025. It is now read-only.

Commit d57d61e

Browse files
Hao Chen (Signals)facebook-github-bot
authored andcommitted
Fix a Flaky Test in CalculatorAppTest (#1919)
Summary: Pull Request resolved: #1919 ## What Modified CalculatorGame's `playFromSecretShare` method to return a dummy item with the publisherBreakdown field populated with dummy items as well, when the number of rows is zero. Added test cases for empty intersection. Added "long_running" and "heavyweight" tags to the test target, to reduce the probability of tests timing out. ## Why A Calculator app test with random input shows as flaky. After investigation, it seems that the test fails when 1) UDP is turned on and 2) the intersection size = 0. In this case, both expected and actual results are dummy, but their formats are different, causing the test to fail. The intersection size = 0 happens with a small probability, which is why the test only fails sometimes. There are also some tests in the suite that requires longer than 600 seconds to run. So I added the tags to increase the timeout limit. ## Investigation process https://docs.google.com/document/d/1raKj9_aaXUED5oCc48ICc5qyf_Anp0Fcy79ZhzV_YTM/edit# Reviewed By: robotal Differential Revision: D41241066 fbshipit-source-id: ba0ecbba728c8f357ab19dfd534bca09796d191e
1 parent 12452c6 commit d57d61e

File tree

4 files changed

+73
-8
lines changed

4 files changed

+73
-8
lines changed

fbpcs/emp_games/lift/pcf2_calculator/CalculatorGame.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,11 @@ class CalculatorGame : public fbpcf::frontend::MpcGame<schedulerId> {
6363
if (inputProcessor.getLiftGameProcessedData().numRows == 0) {
6464
XLOG(WARN) << "skipped calculating as numRows==0.";
6565
// skip game::run(), just output the default metrics.
66-
return GroupedLiftMetrics().toJson();
66+
return GroupedLiftMetrics(
67+
inputProcessor.getLiftGameProcessedData().numPartnerCohorts,
68+
inputProcessor.getLiftGameProcessedData()
69+
.numPublisherBreakdowns)
70+
.toJson();
6771
}
6872

6973
auto attributor = std::make_unique<Attributor<schedulerId>>(

fbpcs/emp_games/lift/pcf2_calculator/input_processing/CompactionBasedInputProcessor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ class CompactionBasedInputProcessor : public IInputProcessor<schedulerId> {
6464
auto intersectionMap = getIntersectionMap(unionMap);
6565

6666
if (intersectionMap.size() == 0) {
67-
liftGameProcessedData_ = {};
67+
liftGameProcessedData_.numRows = 0;
6868
return;
6969
}
7070

fbpcs/emp_games/lift/pcf2_calculator/input_processing/LiftGameProcessedData_impl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ LiftGameProcessedData<schedulerId>::readFromCSV(
141141
testReachShares));
142142

143143
if (result.numRows == 0) {
144-
return LiftGameProcessedData<schedulerId>{};
144+
return result;
145145
}
146146

147147
result.indexShares = transpose(indexShares);

fbpcs/emp_games/lift/pcf2_calculator/test/CalculatorAppTest.cpp

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131

3232
namespace private_lift {
3333

34+
constexpr uint64_t kDefaultNumPublisherBreakdowns = 2;
35+
constexpr uint64_t kDefaultNumCohorts = 4;
36+
3437
template <int schedulerId>
3538
void runCalculatorApp(
3639
int myId,
@@ -356,7 +359,9 @@ GroupedLiftMetrics computeCorrectResults(
356359
bool usingCohorts) {
357360
// Calculate expected results with simple lift calculator
358361
LiftCalculator liftCalculator(
359-
usingCohorts ? 4 : 0, usingPublisherBreakdowns ? 2 : 0, 0);
362+
usingCohorts ? kDefaultNumCohorts : 0,
363+
usingPublisherBreakdowns ? kDefaultNumPublisherBreakdowns : 0,
364+
0);
360365
std::ifstream inFilePublisher{publisherPlaintextInputPath};
361366
std::ifstream inFilePartner{partnerPlaintextInputPath};
362367
int32_t tsOffset = 10;
@@ -386,12 +391,13 @@ void generateSyntheticData(
386391
int numRows,
387392
int numConversionsPerUser,
388393
bool generatePublisherBreakdowns,
389-
bool useCohorts) {
394+
bool useCohorts,
395+
bool forceEmptyIntersection) {
390396
// Generate test input files with random data
391397
GenFakeData testDataGenerator;
392398
LiftFakeDataParams params;
393399
params.setNumRows(numRows)
394-
.setOpportunityRate(0.5)
400+
.setOpportunityRate(forceEmptyIntersection ? 0 : 0.5)
395401
.setTestRate(0.5)
396402
.setPurchaseRate(0.5)
397403
.setIncrementalityRate(0.0)
@@ -426,6 +432,7 @@ TEST_P(CalculatorAppTestFixture, TestCorrectnessRandomInput) {
426432
15,
427433
numConversionsPerUser,
428434
computePublisherBreakdowns,
435+
false,
429436
false);
430437

431438
GroupedLiftMetrics res = readInputFromSecretShares
@@ -462,6 +469,58 @@ TEST_P(CalculatorAppTestFixture, TestCorrectnessRandomInput) {
462469
EXPECT_EQ(expectedResult, res);
463470
}
464471

472+
TEST_P(CalculatorAppTestFixture, TestCorrectnessRandomInputEmptyIntersection) {
473+
// Run calculator app with test input
474+
bool useTls = std::get<0>(GetParam());
475+
bool useXorEncryption = std::get<1>(GetParam());
476+
bool computePublisherBreakdowns = std::get<2>(GetParam());
477+
bool readInputFromSecretShares = std::get<3>(GetParam());
478+
479+
// Generate test input files with random data
480+
int numConversionsPerUser = 25;
481+
generateSyntheticData(
482+
publisherPlaintextInputPath_,
483+
partnerPlaintextInputPath_,
484+
15,
485+
numConversionsPerUser,
486+
computePublisherBreakdowns,
487+
false,
488+
true);
489+
490+
GroupedLiftMetrics res = readInputFromSecretShares
491+
? runUdpTest(
492+
publisherPlaintextInputPath_,
493+
partnerPlaintextInputPath_,
494+
publisherGlobalParamsInputPath_,
495+
partnerGlobalParamsInputPath_,
496+
publisherSecretInputPath_,
497+
partnerSecretInputPath_,
498+
publisherOutputPath_,
499+
partnerOutputPath_,
500+
numConversionsPerUser,
501+
computePublisherBreakdowns,
502+
useTls,
503+
useXorEncryption)
504+
: runTest(
505+
publisherPlaintextInputPath_,
506+
partnerPlaintextInputPath_,
507+
"",
508+
publisherOutputPath_,
509+
partnerOutputPath_,
510+
numConversionsPerUser,
511+
computePublisherBreakdowns,
512+
useTls,
513+
useXorEncryption);
514+
515+
GroupedLiftMetrics expectedResult = computeCorrectResults(
516+
publisherPlaintextInputPath_,
517+
partnerPlaintextInputPath_,
518+
computePublisherBreakdowns,
519+
false);
520+
521+
EXPECT_EQ(expectedResult, res);
522+
}
523+
465524
TEST_P(CalculatorAppTestFixture, TestCorrectnessRandomInputAndCohort) {
466525
// Run calculator app with test input
467526
bool useTls = std::get<0>(GetParam());
@@ -478,7 +537,8 @@ TEST_P(CalculatorAppTestFixture, TestCorrectnessRandomInputAndCohort) {
478537
15,
479538
numConversionsPerUser,
480539
computePublisherBreakdowns,
481-
true);
540+
true,
541+
false);
482542

483543
GroupedLiftMetrics res = readInputFromSecretShares
484544
? runUdpTest(
@@ -530,7 +590,8 @@ TEST_P(CalculatorAppTestFixture, TestWithEmptyInput) {
530590
0,
531591
numConversionsPerUser,
532592
computePublisherBreakdowns,
533-
true);
593+
true,
594+
false);
534595

535596
GroupedLiftMetrics res = readInputFromSecretShares
536597
? runUdpTest(

0 commit comments

Comments
 (0)