Fix caching of winning alternative #329
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… the dashboard is loaded It looks like when this feature was added, the calculation of winning alternatives was meant to take place only once per day. The #calc_winning_alternatives method was never called, which was meant to be saving the experiment's last calc_time. Update the experiment view to call this method instead of the #estimate_winning_alternative method directly. Fix caching so that the #calc_time= method is called, rather than assigning to a local variable. Update calc_time so that number of days since epoch is stored, rather than the day of month (1-31). Ensure we're comparing integer values, rather than the string value Redis returns from #hget.
|
This looks like good catch @nfm, I don't have much free time to investigate the performance issue on the dashboard but maybe you can open a separate issue about it to see if anyone else is available for helping out? |
andrew
added a commit
that referenced
this pull request
Oct 5, 2015
Fix caching of winning alternative
|
Thanks Andrew. I've opened another request and asked Casey to chime in (he wrote the implementation of that feature). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It looks like when this feature was added the calculation of winning alternatives was meant to take place only once per day.
The
#calc_winning_alternativesmethod was never called, which was meant to be saving the experiment's lastcalc_time.Update the experiment view to call this method instead of the
#estimate_winning_alternativemethod directly. Fix caching so that the#calc_time=metreturn value of hod is called, rather than assigning to a local variable. Updatecalc_timeso that number of days since epoch is stored, rather than the day of month (1-31). Ensure we're comparing integer values, rather than the string value Redis returns from#hget.It's worth noting that this pull request changes the value that
#calc_winning_alternativesreturns - it will now returnnilif there was a cache was hit. Given nothing was previously calling this method I think that's probably ok. If not, we can refactor things a little to make it easier to return the cached value.On a side note, I ran into this because our Split dashboard was too slow to load and was timing out on Heroku (30+ second request time with around 40 experiments). I'm not very across the statistical method for calculating the winning alternative - is it possible to speed this up at all, or reduce the number of iterations without compromising the result? It's a relatively expensive operation, and it would be nice to not have to cache the result.
/cc @caser