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

Known future prices don't update possible past price range & small spike strangeness #367

Open
NeatNit opened this issue May 14, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@NeatNit
Copy link

NeatNit commented May 14, 2020

I believe the code that generates small spike is this: https://gist.github.com/Treeki/85be14d297c80c8b3c0a76375743325b#file-turnipprices-cpp-L312-L344

This week I got this pattern up until yesterday: https://turnipprophet.io?prices=108.66.62.58.53.103.147......&pattern=1

Let's take a look at the 3-day peak. In the prediction, it looks like:

  1. 151 to 215
  2. 151 to 216
  3. 151 to 215

But based on the code:

rate = randfloat(1.4, 2.0);
sellPrices[work++] = intceil(randfloat(1.4, rate) * basePrice) - 1;
sellPrices[work++] = intceil(rate * basePrice);
sellPrices[work++] = intceil(randfloat(1.4, rate) * basePrice) - 1;

First issue: The peak itself can be at minimum intceil(1.4 * 108) = 152 - not 151.

Now, today I didn't check in the morning but I did in the afternoon, and got:
https://turnipprophet.io/?prices=108.66.62.58.53.103.147..153....&pattern=1

This confirms that rate is about 1.416, which means that the range for the day before and day after is only 151-152. This is correctly shown for the day after, but not for the day before. Turnip Prophet still shows a possibility for 200+ on Thursday AM.

Probably the code never goes back to fill in the gaps of missing past information.

@NeatNit NeatNit added the bug Something isn't working label May 14, 2020
@peter50216
Copy link
Contributor

Related past PR: #132.

It's a known limitation that the current code doesn't do backward propagation on possible ranges at all.

My take is that this issue is low priority, since the past values are not useful in any way (time travel resets everything).

@NeatNit
Copy link
Author

NeatNit commented May 14, 2020

My take is that this issue is low priority, since the past values are not useful in any way (time travel resets everything).

That's true, but a user who sees this and doesn't know that it's incorrect will be bummed that they missed a peak that never actually occurred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants