Skip to content

Commit

Permalink
Added DCHECKs to find the root cause of a crash in ads::GetParentSegment
Browse files Browse the repository at this point in the history
  • Loading branch information
tmancey committed Oct 13, 2021
1 parent df57072 commit 6687716
Show file tree
Hide file tree
Showing 19 changed files with 274 additions and 97 deletions.
1 change: 1 addition & 0 deletions components/brave_ads/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ source_set("brave_ads_unit_tests") {
"//brave/vendor/bat-native-ads/src/bat/ads/internal/ad_targeting/ad_targeting_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/ad_targeting/ad_targeting_user_model_builder_unittest_util.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/ad_targeting/ad_targeting_user_model_builder_unittest_util.h",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/ad_targeting/ad_targeting_util_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/ad_targeting/processors/behavioral/bandits/epsilon_greedy_bandit_processor_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/ad_targeting/processors/behavioral/purchase_intent/purchase_intent_processor_unittest.cc",
"//brave/vendor/bat-native-ads/src/bat/ads/internal/ad_targeting/processors/contextual/text_classification/text_classification_processor_unittest.cc",
Expand Down
3 changes: 3 additions & 0 deletions vendor/bat-native-ads/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,13 @@ source_set("ads") {
"src/bat/ads/internal/ad_serving/inline_content_ads/inline_content_ad_serving_observer.h",
"src/bat/ads/internal/ad_targeting/ad_targeting.cc",
"src/bat/ads/internal/ad_targeting/ad_targeting.h",
"src/bat/ads/internal/ad_targeting/ad_targeting_constants.h",
"src/bat/ads/internal/ad_targeting/ad_targeting_user_model_builder.cc",
"src/bat/ads/internal/ad_targeting/ad_targeting_user_model_builder.h",
"src/bat/ads/internal/ad_targeting/ad_targeting_user_model_info.cc",
"src/bat/ads/internal/ad_targeting/ad_targeting_user_model_info.h",
"src/bat/ads/internal/ad_targeting/ad_targeting_util.cc",
"src/bat/ads/internal/ad_targeting/ad_targeting_util.h",
"src/bat/ads/internal/ad_targeting/data_types/behavioral/bandits/epsilon_greedy_bandit_arm_info.cc",
"src/bat/ads/internal/ad_targeting/data_types/behavioral/bandits/epsilon_greedy_bandit_arm_info.h",
"src/bat/ads/internal/ad_targeting/data_types/behavioral/bandits/epsilon_greedy_bandit_arms.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <string>

#include "base/check.h"
#include "bat/ads/internal/ad_targeting/data_types/contextual/text_classification/text_classification_aliases.h"
#include "bat/ads/internal/client/client.h"
#include "bat/ads/internal/logging.h"
Expand All @@ -26,6 +27,7 @@ SegmentProbabilitiesMap GetSegmentProbabilities(
for (const auto& probabilities : text_classification_probabilities) {
for (const auto& probability : probabilities) {
const std::string segment = probability.first;
DCHECK(!segment.empty());

const double page_score = probability.second;

Expand Down Expand Up @@ -63,6 +65,8 @@ SegmentList ToSegmentList(

for (const auto& segment_probability : segment_probabilities) {
const std::string segment = segment_probability.first;
DCHECK(!segment.empty());

segments.push_back(segment);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,78 +5,13 @@

#include "bat/ads/internal/ad_targeting/ad_targeting.h"

#include "bat/ads/internal/ad_serving/ad_targeting/models/behavioral/bandits/epsilon_greedy_bandit_model.h"
#include "bat/ads/internal/ad_serving/ad_targeting/models/behavioral/purchase_intent/purchase_intent_model.h"
#include "bat/ads/internal/ad_serving/ad_targeting/models/contextual/text_classification/text_classification_model.h"
#include "bat/ads/internal/ad_targeting/ad_targeting_constants.h"
#include "bat/ads/internal/ad_targeting/ad_targeting_user_model_info.h"
#include "bat/ads/internal/segments/segments_util.h"
#include "bat/ads/internal/ad_targeting/ad_targeting_util.h"

namespace ads {
namespace ad_targeting {

namespace {

const int kTopInterestSegmentsCount = 3;
const int kTopLatentInterestSegmentsCount = 3;
const int kTopPurchaseIntentSegmentsCount = 3;

SegmentList FilterSegments(const SegmentList& segments, const int max_count) {
SegmentList top_segments;

int count = 0;

for (const auto& segment : segments) {
if (ShouldFilterSegment(segment)) {
continue;
}

top_segments.push_back(segment);
count++;
if (count == max_count) {
break;
}
}

return top_segments;
}

SegmentList GetTopSegments(const SegmentList& segments,
const int max_count,
const bool parent_only) {
if (!parent_only) {
return FilterSegments(segments, max_count);
}

const SegmentList parent_segments = GetParentSegments(segments);
return FilterSegments(parent_segments, max_count);
}

SegmentList GetTopSegments(const UserModelInfo& user_model,
const bool parent_only) {
SegmentList segments;

const SegmentList interest_segments = GetTopSegments(
user_model.interest_segments, kTopInterestSegmentsCount, parent_only);
segments.insert(segments.end(), interest_segments.begin(),
interest_segments.end());

const SegmentList latent_interest_segments =
GetTopSegments(user_model.latent_interest_segments,
kTopLatentInterestSegmentsCount, parent_only);
segments.insert(segments.end(), latent_interest_segments.begin(),
latent_interest_segments.end());

const SegmentList purchase_intent_segments =
GetTopSegments(user_model.purchase_intent_segments,
kTopPurchaseIntentSegmentsCount, parent_only);
segments.insert(segments.end(), purchase_intent_segments.begin(),
purchase_intent_segments.end());

return segments;
}

} // namespace

SegmentList GetTopParentChildSegments(const UserModelInfo& user_model) {
return GetTopSegments(user_model, /* parent_only */ false);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_AD_TARGETING_AD_TARGETING_CONSTANTS_H_
#define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_AD_TARGETING_AD_TARGETING_CONSTANTS_H_

namespace ads {
namespace ad_targeting {

const int kTopInterestSegmentsCount = 3;
const int kTopLatentInterestSegmentsCount = 3;
const int kTopPurchaseIntentSegmentsCount = 3;

} // namespace ad_targeting
} // namespace ads

#endif // BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_AD_TARGETING_AD_TARGETING_CONSTANTS_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "bat/ads/internal/ad_targeting/ad_targeting_util.h"

#include "bat/ads/internal/ad_serving/ad_targeting/models/behavioral/bandits/epsilon_greedy_bandit_model.h"
#include "bat/ads/internal/ad_serving/ad_targeting/models/behavioral/purchase_intent/purchase_intent_model.h"
#include "bat/ads/internal/ad_serving/ad_targeting/models/contextual/text_classification/text_classification_model.h"
#include "bat/ads/internal/ad_targeting/ad_targeting_constants.h"
#include "bat/ads/internal/ad_targeting/ad_targeting_user_model_info.h"
#include "bat/ads/internal/segments/segments_util.h"

namespace ads {
namespace ad_targeting {

namespace {

SegmentList FilterSegments(const SegmentList& segments, const int max_count) {
SegmentList top_segments;

int count = 0;

for (const auto& segment : segments) {
if (ShouldFilterSegment(segment)) {
continue;
}

top_segments.push_back(segment);
count++;
if (count == max_count) {
break;
}
}

return top_segments;
}

} // namespace

SegmentList GetTopSegments(const SegmentList& segments,
const int max_count,
const bool parent_only) {
if (!parent_only) {
return FilterSegments(segments, max_count);
}

const SegmentList parent_segments = GetParentSegments(segments);
return FilterSegments(parent_segments, max_count);
}

SegmentList GetTopSegments(const UserModelInfo& user_model,
const bool parent_only) {
SegmentList segments;

const SegmentList interest_segments = GetTopSegments(
user_model.interest_segments, kTopInterestSegmentsCount, parent_only);
segments.insert(segments.end(), interest_segments.begin(),
interest_segments.end());

const SegmentList latent_interest_segments =
GetTopSegments(user_model.latent_interest_segments,
kTopLatentInterestSegmentsCount, parent_only);
segments.insert(segments.end(), latent_interest_segments.begin(),
latent_interest_segments.end());

const SegmentList purchase_intent_segments =
GetTopSegments(user_model.purchase_intent_segments,
kTopPurchaseIntentSegmentsCount, parent_only);
segments.insert(segments.end(), purchase_intent_segments.begin(),
purchase_intent_segments.end());

return segments;
}

} // namespace ad_targeting
} // namespace ads
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_AD_TARGETING_AD_TARGETING_UTIL_H_
#define BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_AD_TARGETING_AD_TARGETING_UTIL_H_

#include "bat/ads/internal/segments/segments_aliases.h"

namespace ads {
namespace ad_targeting {

struct UserModelInfo;

SegmentList GetTopSegments(const SegmentList& segments,
const int max_count,
const bool parent_only);

SegmentList GetTopSegments(const UserModelInfo& user_model,
const bool parent_only);

} // namespace ad_targeting
} // namespace ads

#endif // BRAVE_VENDOR_BAT_NATIVE_ADS_SRC_BAT_ADS_INTERNAL_AD_TARGETING_AD_TARGETING_UTIL_H_
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#include "bat/ads/internal/ad_targeting/ad_targeting_util.h"

#include "bat/ads/internal/ad_targeting/ad_targeting_user_model_info.h"
#include "bat/ads/internal/unittest_base.h"

// npm run test -- brave_unit_tests --filter=BatAds*

namespace ads {
namespace ad_targeting {

class BatAdsAdTargetingUtilTest : public UnitTestBase {
protected:
BatAdsAdTargetingUtilTest() = default;

~BatAdsAdTargetingUtilTest() override = default;
};

TEST_F(BatAdsAdTargetingUtilTest, GetTopParentChildSegments) {
// Arrange
UserModelInfo user_model;
user_model.interest_segments = {"interest-1", "interest-2"};
user_model.latent_interest_segments = {"latent_interest-1",
"latent_interest-2"};
user_model.purchase_intent_segments = {"purchase_intent-1",
"purchase_intent-2"};

// Act
const SegmentList segments =
GetTopSegments(user_model, /* parent_only */ false);

// Assert
const SegmentList expected_segments = {
"interest-1", "interest-2", "latent_interest-1",
"latent_interest-2", "purchase_intent-1", "purchase_intent-2"};

EXPECT_EQ(expected_segments, segments);
}

TEST_F(BatAdsAdTargetingUtilTest, GetTopParentChildSegmentsForEmptyUserModel) {
// Arrange
const UserModelInfo user_model;

// Act
const SegmentList segments =
GetTopSegments(user_model, /* parent_only */ false);

// Assert
EXPECT_TRUE(segments.empty());
}

TEST_F(BatAdsAdTargetingUtilTest, GetTopParentSegments) {
// Arrange
UserModelInfo user_model;
user_model.interest_segments = {"interest_1", "interest_2"};
user_model.latent_interest_segments = {"latent_interest_1",
"latent_interest_2"};
user_model.purchase_intent_segments = {"purchase_intent_1",
"purchase_intent_2"};

// Act
const SegmentList segments =
GetTopSegments(user_model, /* parent_only */ true);

// Assert
const SegmentList expected_segments = {
"interest_1", "interest_2", "latent_interest_1",
"latent_interest_2", "purchase_intent_1", "purchase_intent_2"};

EXPECT_EQ(expected_segments, segments);
}

TEST_F(BatAdsAdTargetingUtilTest, GetTopParentSegmentsForEmptyUserModel) {
// Arrange
const UserModelInfo user_model;

// Act
const SegmentList segments =
GetTopSegments(user_model, /* parent_only */ true);

// Assert
EXPECT_TRUE(segments.empty());
}

} // namespace ad_targeting
} // namespace ads
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ bool GetArmFromDictionary(const base::DictionaryValue* dictionary,
return false;
}
arm.segment = *segment;
DCHECK(!arm.segment.empty());

arm.pulls = dictionary->FindIntKey(kPullsKey).value_or(0);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include <algorithm>

#include "base/check.h"
#include "base/notreached.h"
#include "bat/ads/ads_client.h"
#include "bat/ads/internal/ad_targeting/data_types/behavioral/bandits/epsilon_greedy_bandit_arms.h"
Expand Down Expand Up @@ -82,7 +83,10 @@ EpsilonGreedyBandit::EpsilonGreedyBandit() {
EpsilonGreedyBandit::~EpsilonGreedyBandit() = default;

void EpsilonGreedyBandit::Process(const BanditFeedbackInfo& feedback) {
DCHECK(!feedback.segment.empty());

const std::string segment = GetParentSegment(feedback.segment);
DCHECK(!segment.empty());

switch (feedback.ad_event_type) {
case mojom::AdNotificationEventType::kTimedOut:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ namespace {

std::string GetTopSegmentFromPageProbabilities(
const TextClassificationProbabilitiesMap& probabilities) {
if (probabilities.empty()) {
return "";
}
DCHECK(!probabilities.empty());

const auto iter =
std::max_element(probabilities.begin(), probabilities.end(),
Expand Down Expand Up @@ -61,6 +59,7 @@ void TextClassification::Process(const std::string& text) {
}

const std::string segment = GetTopSegmentFromPageProbabilities(probabilities);
DCHECK(!segment.empty());
BLOG(1, "Classified text with the top segment as " << segment);

Client::Get()->AppendTextClassificationProbabilitiesToHistory(probabilities);
Expand Down
Loading

0 comments on commit 6687716

Please sign in to comment.