Skip to content

Conversation

@jowlee
Copy link
Contributor

@jowlee jowlee commented Oct 29, 2021

Description

Edge cases for trending version aSPET not matching ePWJD.
Two bugs:
1.) Previously we were getting the diff in days in python as follows:

    o = track["created_at"]
    K = datetime.now()
    w = parse(o)
    k = (K - w).days

which gets the total diff in days, seconds, milliseconds and we take the day part off it.
In the updated postgres version, we get the diff by running (now()::date - aip.created_at::date) which gets the actual dates and subtracts the day part of it ie. 09/25/2021 - 09/21/2021 => 4 days

The previous case takes into consideration elapsed time, so if 24 hours past it counts as a day while the postgres method only takes into consideration the date.
To fix this, the day diff is not calculated using EXTRACT(DAYS from now() - aip.created_at)

2.) When fetching tracks to cache into the top trending, the selected tracks to score are the top most played in that time range. The net multiplier was not used in selecting the tracks in the new mat view method.

Tests

Serval scripts were written to compare the results from the two trending methods

How will this change be monitored?

manually

@jowlee jowlee added bug Something isn't working discovery-node Discovery Node (previously known as Discovery Provider) labels Oct 29, 2021
)
top_listen_tracks_subquery = top_listen_tracks_subquery.limit(limit)

score_params = strategy.get_score_params()
Copy link
Member

Choose a reason for hiding this comment

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

oh yes, the net multiplier is good

Copy link
Member

Choose a reason for hiding this comment

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

do we need the net multiplier at all though? can we issue this request without a top listen subquery?

Copy link
Member

Choose a reason for hiding this comment

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

in line w/ the "one experiment" thing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This top listen tracks with the net multiplier is strictly to keep the trending consistent with the current trending algorithm.
The current trending gets the top X (100* net_multiplier) tracks and scores / sorts those and then takes the top 100.
This new trending scheme with the DB track trending score does not need this at all. It was previously done as a heuristic to make it more performant, but we do it to keep the trending consistent.

@jowlee jowlee force-pushed the jowlee-fix-trending branch from c5d928f to 653d858 Compare November 3, 2021 15:35
@jowlee jowlee merged commit 2d3f405 into master Nov 3, 2021
@jowlee jowlee deleted the jowlee-fix-trending branch November 3, 2021 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working discovery-node Discovery Node (previously known as Discovery Provider)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants