Skip to content

Commit

Permalink
Merge pull request #729 from readthedocs/davidfischer/refactor_today_…
Browse files Browse the repository at this point in the history
…methods

Refactor the names of `*_today` methods
  • Loading branch information
davidfischer committed Apr 11, 2023
2 parents ff54ab8 + f9fbabc commit c7466ed
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 59 deletions.
10 changes: 5 additions & 5 deletions adserver/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,8 +550,8 @@ class FlightAdmin(RemoveDeleteMixin, FlightMixin, SimpleHistoryAdmin):
"cpc",
"cpm",
"value_remaining",
"clicks_needed_today",
"views_needed_today",
"clicks_needed_this_interval",
"views_needed_this_interval",
"priority_multiplier",
"total_clicks",
"total_views",
Expand All @@ -575,9 +575,9 @@ class FlightAdmin(RemoveDeleteMixin, FlightMixin, SimpleHistoryAdmin):
"total_views",
"clicks_today",
"views_today",
"clicks_needed_today",
"views_needed_today",
"weighted_clicks_needed_today",
"clicks_needed_this_interval",
"views_needed_this_interval",
"weighted_clicks_needed_this_interval",
"modified",
"created",
)
Expand Down
16 changes: 9 additions & 7 deletions adserver/decisionengine/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ def filter_flight(self, flight):
return False

# Skip if there are no clicks or views needed today/this interval (ad pacing)
if flight.weighted_clicks_needed_today() <= 0:
if flight.weighted_clicks_needed_this_interval() <= 0:
return False

return True
Expand Down Expand Up @@ -372,24 +372,26 @@ def select_flight(self):
# to the possible list of flights
if any(
(
(flight.clicks_needed_today() > 0),
(flight.views_needed_today() > 0),
(flight.clicks_needed_this_interval() > 0),
(flight.views_needed_this_interval() > 0),
)
):
# NOTE: takes into account views for CPM ads
# Takes eCPM (CTR * CPC for CPC ads) into account
weighted_clicks_needed_today = flight.weighted_clicks_needed_today(
self.publisher,
weighted_clicks_needed_this_interval = (
flight.weighted_clicks_needed_this_interval(
self.publisher,
)
)

flight_range.append(
[
total_clicks_needed,
total_clicks_needed + weighted_clicks_needed_today,
total_clicks_needed + weighted_clicks_needed_this_interval,
flight,
]
)
total_clicks_needed += weighted_clicks_needed_today
total_clicks_needed += weighted_clicks_needed_this_interval

choice = random.randint(0, total_clicks_needed)
for min_clicks, max_clicks, flight in flight_range:
Expand Down
10 changes: 5 additions & 5 deletions adserver/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1079,7 +1079,7 @@ def clicks_today(self):
# The aggregation can be `None` if there are no impressions
return aggregation or 0

def views_needed_today(self):
def views_needed_this_interval(self):
if (
not self.live
or self.views_remaining() <= 0
Expand All @@ -1097,7 +1097,7 @@ def views_needed_today(self):

return self.views_remaining()

def clicks_needed_today(self):
def clicks_needed_this_interval(self):
"""Calculates clicks needed based on the impressions this flight's ads have."""
if (
not self.live
Expand All @@ -1116,7 +1116,7 @@ def clicks_needed_today(self):

return self.clicks_remaining()

def weighted_clicks_needed_today(self, publisher=None):
def weighted_clicks_needed_this_interval(self, publisher=None):
"""
Calculates clicks needed taking into account a flight's priority.
Expand All @@ -1128,8 +1128,8 @@ def weighted_clicks_needed_today(self, publisher=None):
impressions_needed = 0

# This is naive but we are counting a click as being worth 1,000 views
impressions_needed += math.ceil(self.views_needed_today() / 1000.0)
impressions_needed += self.clicks_needed_today()
impressions_needed += math.ceil(self.views_needed_this_interval() / 1000.0)
impressions_needed += self.clicks_needed_this_interval()

if self.cpc:
# Use the publisher CTR if available
Expand Down
77 changes: 44 additions & 33 deletions adserver/tests/test_decision_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,20 +213,22 @@ def test_custom_interval(self):
self.assertEqual(self.cpm_flight.sold_clicks, 0)
self.assertEqual(self.cpm_flight.sold_impressions, 10_000)
self.assertEqual(self.cpm_flight.sold_days(), 31 * 24)
self.assertEqual(self.cpm_flight.clicks_needed_today(), 0)
self.assertEqual(self.cpm_flight.views_needed_today(), 10_000 - pace)
self.assertEqual(self.cpm_flight.clicks_needed_this_interval(), 0)
self.assertEqual(
self.cpm_flight.views_needed_this_interval(), 10_000 - pace
)

self.assertTrue(self.backend.filter_flight(self.cpm_flight))

self.cpm_flight.total_views = 10_000 - pace - 1
self.cpm_flight.save()
self.assertEqual(self.cpm_flight.views_needed_today(), 1)
self.assertEqual(self.cpm_flight.views_needed_this_interval(), 1)
self.assertTrue(self.backend.filter_flight(self.cpm_flight))

# Don't show this flight anymore. It is above pace
self.cpm_flight.total_views = 10_000 - pace
self.cpm_flight.save()
self.assertEqual(self.cpm_flight.views_needed_today(), 0)
self.assertEqual(self.cpm_flight.views_needed_this_interval(), 0)
self.assertFalse(self.backend.filter_flight(self.cpm_flight))

def test_flight_clicks(self):
Expand Down Expand Up @@ -345,7 +347,7 @@ def test_flight_mobile_targeting(self):
self.assertIsNone(ad)

def test_clicks_needed(self):
self.assertEqual(self.include_flight.clicks_needed_today(), 33)
self.assertEqual(self.include_flight.clicks_needed_this_interval(), 33)

clicks_to_simulate = 10
for _ in range(clicks_to_simulate):
Expand All @@ -354,12 +356,12 @@ def test_clicks_needed(self):
# Refresh the data on the include_flight - gets the denormalized views
self.include_flight.refresh_from_db()

self.assertEqual(self.include_flight.clicks_needed_today(), 23)
self.assertEqual(self.include_flight.clicks_needed_this_interval(), 23)

# Set to a date in the past
self.include_flight.end_date = get_ad_day().date() - datetime.timedelta(days=2)
self.assertEqual(
self.include_flight.clicks_needed_today(),
self.include_flight.clicks_needed_this_interval(),
self.include_flight.sold_clicks - clicks_to_simulate,
)

Expand All @@ -368,9 +370,9 @@ def test_views_needed(self):
self.advertisement1.flight = self.cpm_flight
self.advertisement1.save()

self.assertEqual(self.cpm_flight.clicks_needed_today(), 0)
self.assertEqual(self.cpm_flight.clicks_needed_this_interval(), 0)
# 0% through the flight, 31 days
self.assertEqual(self.cpm_flight.views_needed_today(), 323)
self.assertEqual(self.cpm_flight.views_needed_this_interval(), 323)

views_to_simulate = 10
for _ in range(views_to_simulate):
Expand All @@ -379,12 +381,12 @@ def test_views_needed(self):
# Refresh the data on the include_flight - gets the denormalized views
self.cpm_flight.refresh_from_db()

self.assertEqual(self.cpm_flight.views_needed_today(), 313)
self.assertEqual(self.cpm_flight.views_needed_this_interval(), 313)

# Set to a date in the past
self.cpm_flight.end_date = get_ad_day().date() - datetime.timedelta(days=2)
self.assertEqual(
self.cpm_flight.views_needed_today(),
self.cpm_flight.views_needed_this_interval(),
self.cpm_flight.sold_impressions - views_to_simulate,
)

Expand Down Expand Up @@ -435,8 +437,8 @@ def test_click_probability(self):
flight1.save()
flight2.save()

flight1_prob = flight1.weighted_clicks_needed_today()
flight2_prob = flight2.weighted_clicks_needed_today()
flight1_prob = flight1.weighted_clicks_needed_this_interval()
flight2_prob = flight2.weighted_clicks_needed_this_interval()
total = flight1_prob + flight2_prob

with mock.patch("random.randint") as randint:
Expand Down Expand Up @@ -731,16 +733,16 @@ def test_ctr_weighting(self):
)

# Add bonus probability for good performance
self.assertEqual(good_ctr.clicks_needed_today(), 28)
self.assertEqual(good_ctr.clicks_needed_this_interval(), 28)
weighting_boost = 10 * 2.5 * 0.333 # 10 (constant) * 2.5 (cpc) * 0.333 (ctr)
self.assertEqual(
good_ctr.weighted_clicks_needed_today(), int(28 * weighting_boost)
good_ctr.weighted_clicks_needed_this_interval(), int(28 * weighting_boost)
)

# Don't allow down-weighting for bad performance,
# only add bonus for good performance
self.assertEqual(bad_ctr.clicks_needed_today(), 33)
self.assertEqual(bad_ctr.weighted_clicks_needed_today(), 33)
self.assertEqual(bad_ctr.clicks_needed_this_interval(), 33)
self.assertEqual(bad_ctr.weighted_clicks_needed_this_interval(), 33)

def test_cpm_weighting(self):
# Remove existing flights
Expand Down Expand Up @@ -775,16 +777,21 @@ def test_cpm_weighting(self):

self.publisher.sampled_ctr = 0.2

self.assertEqual(low_cost.clicks_needed_today(), 28)
self.assertEqual(low_cost.weighted_clicks_needed_today(), 28)
self.assertEqual(low_cost.clicks_needed_this_interval(), 28)
self.assertEqual(low_cost.weighted_clicks_needed_this_interval(), 28)
# Publisher CTR has no effect
self.assertEqual(low_cost.weighted_clicks_needed_today(self.publisher), 28)
self.assertEqual(
low_cost.weighted_clicks_needed_this_interval(self.publisher), 28
)

self.assertEqual(high_cost.clicks_needed_today(), 28)
self.assertEqual(high_cost.weighted_clicks_needed_today(), 28 * high_cost.cpm)
self.assertEqual(high_cost.clicks_needed_this_interval(), 28)
self.assertEqual(
high_cost.weighted_clicks_needed_this_interval(), 28 * high_cost.cpm
)
# Publisher CTR has no effect
self.assertEqual(
high_cost.weighted_clicks_needed_today(self.publisher), 28 * high_cost.cpm
high_cost.weighted_clicks_needed_this_interval(self.publisher),
28 * high_cost.cpm,
)

def test_publisher_weighting_bonus(self):
Expand All @@ -806,21 +813,21 @@ def test_publisher_weighting_bonus(self):
pacing_interval=24 * 60 * 60,
)

self.assertEqual(flight.clicks_needed_today(), 33)
self.assertEqual(flight.weighted_clicks_needed_today(), 33)
self.assertEqual(flight.clicks_needed_this_interval(), 33)
self.assertEqual(flight.weighted_clicks_needed_this_interval(), 33)

# Check publisher weighting
self.publisher.sampled_ctr = 0.2
weighting_bonus = 0.2 * 2.25 * 10 # (ctr) * (cpc) * (constant)
self.assertEqual(
flight.weighted_clicks_needed_today(self.publisher),
flight.weighted_clicks_needed_this_interval(self.publisher),
int(33 * weighting_bonus),
)

self.publisher.sampled_ctr = 2 # VERY high CTR
weighting_bonus = 10 # capped at 10
self.assertEqual(
flight.weighted_clicks_needed_today(self.publisher),
flight.weighted_clicks_needed_this_interval(self.publisher),
int(33 * weighting_bonus),
)

Expand Down Expand Up @@ -868,15 +875,19 @@ def test_weighting_bounds(self):
)

# 1x
self.assertEqual(super_low.clicks_needed_today(), 33)
self.assertEqual(super_low.weighted_clicks_needed_today(), 33 * super_low.cpm)
self.assertEqual(super_low.clicks_needed_this_interval(), 33)
self.assertEqual(
super_low.weighted_clicks_needed_this_interval(), 33 * super_low.cpm
)

# 2.5x
self.assertEqual(high.clicks_needed_today(), 28)
self.assertEqual(high.weighted_clicks_needed_today(), int(28 * high.cpm))
self.assertEqual(high.clicks_needed_this_interval(), 28)
self.assertEqual(
high.weighted_clicks_needed_this_interval(), int(28 * high.cpm)
)

# 10x
self.assertEqual(super_high.clicks_needed_today(), 28)
self.assertEqual(super_high.clicks_needed_this_interval(), 28)
self.assertEqual(
super_high.weighted_clicks_needed_today(), 28 * 10
super_high.weighted_clicks_needed_this_interval(), 28 * 10
) # maxed out
18 changes: 9 additions & 9 deletions adserver/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ def test_start_date_math(self):

self.assertEqual(self.flight.days_remaining(), 16)
# 15/31% through the flight, 0% through the clicks
self.assertEqual(self.flight.clicks_needed_today(), 484)
self.assertEqual(self.flight.clicks_needed_this_interval(), 484)

self.flight.start_date = get_ad_day().date()
self.flight.end_date = self.flight.start_date + datetime.timedelta(days=30)
Expand All @@ -202,22 +202,22 @@ def test_start_date_math(self):
self.flight.sold_clicks = 1000
self.assertEqual(self.flight.days_remaining(), 30)
self.assertEqual(self.flight.clicks_remaining(), 1000)
self.assertEqual(self.flight.clicks_needed_today(), 33)
self.assertEqual(self.flight.clicks_needed_this_interval(), 33)

self.flight.sold_clicks = 950
self.assertEqual(self.flight.clicks_needed_today(), 31)
self.assertEqual(self.flight.clicks_needed_this_interval(), 31)

self.flight.sold_clicks = 0
self.assertEqual(self.flight.clicks_needed_today(), 0)
self.assertEqual(self.flight.clicks_needed_this_interval(), 0)

self.flight.sold_impressions = 10000
# 0% through the views, 31 days
self.assertEqual(self.flight.views_needed_today(), 323)
self.assertEqual(self.flight.views_needed_this_interval(), 323)

self.flight.start_date = get_ad_day().date() - datetime.timedelta(days=15)
self.flight.end_date = self.flight.start_date + datetime.timedelta(days=30)
# 16/31% through the flight, 0% through the views
self.assertEqual(self.flight.views_needed_today(), 5162)
self.assertEqual(self.flight.views_needed_this_interval(), 5162)

def test_custom_interval(self):
now = get_ad_day()
Expand All @@ -243,7 +243,7 @@ def test_custom_interval(self):

self.assertEqual(self.flight.days_remaining(), 31 * 24 - 1)
self.assertEqual(
self.flight.clicks_needed_today(),
self.flight.clicks_needed_this_interval(),
self.flight.sold_clicks - pace,
)

Expand All @@ -260,13 +260,13 @@ def test_custom_interval(self):
# So we need all the clicks from the previous 14 days
# plus whatever is expected in the current interval
self.assertEqual(
self.flight.clicks_needed_today(),
self.flight.clicks_needed_this_interval(),
self.flight.sold_clicks - pace,
)

# Make the flight overfulfilled
self.flight.total_clicks = int(self.flight.sold_clicks * 0.80)
self.assertEqual(self.flight.clicks_needed_today(), 0)
self.assertEqual(self.flight.clicks_needed_this_interval(), 0)

def test_render_ad(self):
ad_type1 = get(
Expand Down

0 comments on commit c7466ed

Please sign in to comment.