Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes AC for 1min setting #1507

Merged
merged 4 commits into from
Feb 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ test("brave_unit_tests") {
sources += [
"//brave/vendor/bat-native-ledger/src/bat_get_media_unittest.cc",
"//brave/vendor/bat-native-ledger/src/bat_helper_unittest.cc",
"//brave/vendor/bat-native-ledger/src/bat_publishers_unittest.cc",
"//brave/vendor/bat-native-ledger/src/test/niceware_partial_unittest.cc",
"//brave/components/brave_rewards/browser/publisher_info_database_unittest.cc",
"//brave/vendor/bat-native-usermodel/test/usermodel_unittest.cc",
Expand Down
12 changes: 6 additions & 6 deletions vendor/bat-native-ledger/src/bat_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ static bool ignore_ = false;
allow_videos_ = state.allow_videos_;
monthly_balances_ = state.monthly_balances_;
recurring_donation_ = state.recurring_donation_;
migrate_score = state.migrate_score;
migrate_score_2 = state.migrate_score_2;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

score_2?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we migrated once already and we just need to migrate it again, so that's why I rename it to 2 as 1 is not needed anymore

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but why does it need to be score_2 in the data structure?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so that we migrate only once

Copy link
Collaborator

@bridiver bridiver Feb 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the name has changed now, but I think what we need here is a version attribute for the data

}

PUBLISHER_STATE_ST::~PUBLISHER_STATE_ST() {}
Expand Down Expand Up @@ -670,10 +670,10 @@ static bool ignore_ = false;
}
}

if (d.HasMember("migrate_score") && d["migrate_score"].IsBool()) {
migrate_score = d["migrate_score"].GetBool();
if (d.HasMember("migrate_score_2") && d["migrate_score_2"].IsBool()) {
migrate_score_2 = d["migrate_score_2"].GetBool();
} else {
migrate_score = true;
migrate_score_2 = true;
}
}

Expand Down Expand Up @@ -722,8 +722,8 @@ static bool ignore_ = false;

writer.EndArray();

writer.String("migrate_score");
writer.Bool(data.migrate_score);
writer.String("migrate_score_2");
writer.Bool(data.migrate_score_2);

writer.EndObject();
}
Expand Down
2 changes: 1 addition & 1 deletion vendor/bat-native-ledger/src/bat_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ namespace braveledger_bat_helper {
bool allow_videos_ = true;
std::map<std::string, REPORT_BALANCE_ST> monthly_balances_;
std::map<std::string, double> recurring_donation_;
bool migrate_score = false;
bool migrate_score_2 = false;
};

struct PUBLISHER_ST {
Expand Down
23 changes: 13 additions & 10 deletions vendor/bat-native-ledger/src/bat_publishers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,22 @@ BatPublishers::BatPublishers(bat_ledger::LedgerImpl* ledger):
BatPublishers::~BatPublishers() {
}

void BatPublishers::calcScoreConsts(const uint64_t& min_duration) {
uint64_t min_duration_ms = min_duration *
braveledger_ledger::_milliseconds_second;
a_ = (1.0 / (braveledger_ledger::_d * 2.0)) - min_duration_ms;
void BatPublishers::calcScoreConsts(const uint64_t& min_duration_seconds) {
// we increase duration for 100 to keep it as close to muon implementation
// as possible (we used 1000 in muon)
// keeping it with only seconds visits are not spaced out equally
uint64_t min_duration_big = min_duration_seconds * 100;
a_ = (1.0 / (braveledger_ledger::_d * 2.0)) - min_duration_big;
a2_ = a_ * 2.0;
a4_ = a2_ * 2.0;
b_ = min_duration_ms - a_;
b_ = min_duration_big - a_;
b2_ = b_ * b_;
}

// courtesy of @dimitry-xyz: https://github.com/brave/ledger/issues/2#issuecomment-221752002
double BatPublishers::concaveScore(const uint64_t& duration) {
uint64_t duration_ms = duration * braveledger_ledger::_milliseconds_second;
return (-b_ + std::sqrt(b2_ + (a4_ * duration_ms))) / a2_;
double BatPublishers::concaveScore(const uint64_t& duration_seconds) {
uint64_t duration_big = duration_seconds * 100;
return (-b_ + std::sqrt(b2_ + (a4_ * duration_big))) / a2_;
}

std::string getProviderName(const std::string& publisher_id) {
Expand Down Expand Up @@ -431,6 +433,7 @@ void BatPublishers::OnRestorePublishersInternal(bool success) {

void BatPublishers::setPublisherMinVisitTime(const uint64_t& duration) { // In seconds
state_->min_publisher_duration_ = duration;
calcScoreConsts(duration);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to update variables on the fly and not only when we restart the browser

Copy link
Collaborator

@bridiver bridiver Feb 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should really move these things to an observer for PublisherMinVisitTime change in a follow-up somewhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so ledger would save data and call rewards that save was successful and we would trigger observer that would then call back ledger?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, an observer internal to ledger

SynopsisNormalizer();
saveState();
}
Expand Down Expand Up @@ -488,11 +491,11 @@ bool BatPublishers::getPublisherAllowVideos() const {
}

bool BatPublishers::GetMigrateScore() const {
return state_->migrate_score;
return state_->migrate_score_2;
}

void BatPublishers::SetMigrateScore(bool value) {
state_->migrate_score = value;
state_->migrate_score_2 = value;
saveState();
}

Expand Down
10 changes: 8 additions & 2 deletions vendor/bat-native-ledger/src/bat_publishers.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "bat/ledger/ledger_callback_handler.h"
#include "bat/ledger/publisher_info.h"
#include "bat_helper.h"
#include "base/gtest_prod_util.h"

namespace bat_ledger {
class LedgerImpl;
Expand Down Expand Up @@ -174,9 +175,9 @@ class BatPublishers : public ledger::LedgerCallbackHandler {

void OnRestorePublishersInternal(bool success);

void calcScoreConsts(const uint64_t& duration);
void calcScoreConsts(const uint64_t& min_duration_seconds);

double concaveScore(const uint64_t& duration);
double concaveScore(const uint64_t& duration_seconds);

void saveState();

Expand Down Expand Up @@ -221,6 +222,11 @@ class BatPublishers : public ledger::LedgerCallbackHandler {
double b_;

double b2_;

// For testing purposes
friend class BatPublishersTest;
FRIEND_TEST_ALL_PREFIXES(BatPublishersTest, calcScoreConsts);
FRIEND_TEST_ALL_PREFIXES(BatPublishersTest, concaveScore);
};

} // namespace braveledger_bat_publishers
Expand Down
98 changes: 98 additions & 0 deletions vendor/bat-native-ledger/src/bat_publishers_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/* 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/ledger/ledger.h"
#include "bat_publishers.h"
#include "testing/gtest/include/gtest/gtest.h"

// npm run test -- brave_unit_tests --filter=BatPublishersTest.*

namespace braveledger_bat_publishers {

class BatPublishersTest : public testing::Test {
};

TEST_F(BatPublishersTest, calcScoreConsts) {
braveledger_bat_publishers::BatPublishers* publishers =
new braveledger_bat_publishers::BatPublishers(nullptr);

/*
* Test 5 seconds
*/
publishers->calcScoreConsts(5);

EXPECT_EQ(publishers->a_, 14500);
EXPECT_EQ(publishers->a2_, 29000);
EXPECT_EQ(publishers->a4_, 58000);
EXPECT_EQ(publishers->b_, -14000);
EXPECT_EQ(publishers->b2_, 196000000);

/*
* Test 8 seconds
*/
publishers->calcScoreConsts(8);

EXPECT_EQ(publishers->a_, 14200);
EXPECT_EQ(publishers->a2_, 28400);
EXPECT_EQ(publishers->a4_, 56800);
EXPECT_EQ(publishers->b_, -13400);
EXPECT_EQ(publishers->b2_, 179560000);

/*
* Test 1min (60 seconds)
*/
publishers->calcScoreConsts(60);

EXPECT_EQ(publishers->a_, 9000);
EXPECT_EQ(publishers->a2_, 18000);
EXPECT_EQ(publishers->a4_, 36000);
EXPECT_EQ(publishers->b_, -3000);
EXPECT_EQ(publishers->b2_, 9000000);
}

TEST_F(BatPublishersTest, concaveScore) {
braveledger_bat_publishers::BatPublishers* publishers =
new braveledger_bat_publishers::BatPublishers(nullptr);

/*
* min duration: 5 seconds
* duration: 5, 15, 60, 1000, 10000, 150000, 500000
*/
publishers->calcScoreConsts(5);
EXPECT_NEAR(publishers->concaveScore(5), 1, 0.001f);
EXPECT_NEAR(publishers->concaveScore(15), 1.06285, 0.001f);
EXPECT_NEAR(publishers->concaveScore(60), 1.28703, 0.001f);
EXPECT_NEAR(publishers->concaveScore(1000), 3.15289, 0.001f);
EXPECT_NEAR(publishers->concaveScore(10000), 8.80133, 0.001f);
EXPECT_NEAR(publishers->concaveScore(150000), 32.6498, 0.001f);
EXPECT_NEAR(publishers->concaveScore(500000), 59.2068, 0.001f);

/*
* min duration: 8 seconds
* duration: 5, 15, 60, 1000, 10000, 150000, 500000
*/
publishers->calcScoreConsts(8);
EXPECT_NEAR(publishers->concaveScore(5), 0.979606, 0.001f);
EXPECT_NEAR(publishers->concaveScore(15), 1.04477, 0.001f);
EXPECT_NEAR(publishers->concaveScore(60), 1.27505, 0.001f);
EXPECT_NEAR(publishers->concaveScore(1000), 3.16717, 0.001f);
EXPECT_NEAR(publishers->concaveScore(10000), 8.8769, 0.001f);
EXPECT_NEAR(publishers->concaveScore(150000), 32.9766, 0.001f);
EXPECT_NEAR(publishers->concaveScore(500000), 59.8128, 0.001f);

/*
* min duration: 60 seconds
* duration: 5, 15, 60, 1000, 10000, 150000, 500000
*/
publishers->calcScoreConsts(60);
EXPECT_NEAR(publishers->concaveScore(5), 0.455342, 0.001f);
EXPECT_NEAR(publishers->concaveScore(15), 0.607625, 0.001f);
EXPECT_NEAR(publishers->concaveScore(60), 1, 0.001f);
EXPECT_NEAR(publishers->concaveScore(1000), 3.50416, 0.001f);
EXPECT_NEAR(publishers->concaveScore(10000), 10.7089, 0.001f);
EXPECT_NEAR(publishers->concaveScore(150000), 40.9918, 0.001f);
EXPECT_NEAR(publishers->concaveScore(500000), 74.7025, 0.001f);
}

}