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

Skatesum uses final rate of graph matrix, which is sometimes totally wrong #147

Open
4 tasks
dreeves opened this issue Aug 20, 2020 · 1 comment
Open
4 tasks

Comments

@dreeves
Copy link
Member

dreeves commented Aug 20, 2020

Desiderata

Preview Give feedback

HT Grayson:

Aggday skatesum takes only the final rate into consideration when it decides what the cap value is for the plot. If your goal will do anything fancy at all -- anything other than stay at the rate you gave it when you started it -- then the graph will break when you change things and derails will boom into existence.

Skatesum is not officially supported. If you want your graph to have breaks, rate changes, or other not-the-same-rate-everywhere-forever, then skatesum won't work the way you're wanting it to.

Above is from a forum thread.

This is a nontrivial fix -- we can't just change rfin to rcur due to a circular dependency.

See line 592 of beebrain.js:

// HACK: aggday=skatesum needs to know rcur which we won't know until we do
// procParams. We do know rfin so we're making do with that for now...
br.rsk8 = gol.rfin * SID / gol.siru // convert rfin to daily rate

Cognata

Verbata: skatesum aggday, #baneful bug,

@dreeves
Copy link
Member Author

dreeves commented Dec 6, 2020

I'm not sure if this will be an easy fix -- changing rfin to rcur -- but let's find out!

Oof, this is easier said than done!

Aggdaying happens in procData.
But aggday=skatesum needs to know the current daily rate.
But that's based on rcur which is computed in procParams.
And we do procData before procParams; we don't know rcur yet in procData.
I'm thinking we can get rcur in procRoad and that can happen before procData.

May require thoughtful refactoring.

I did fix a different bug with skatesum along the way: #169

PS: This was broken in Pybrain too so does not count as a zombie.

@dreeves dreeves removed the ZOM Regression label Dec 6, 2020
@dreeves dreeves removed PEA Easy-peasy SKY Pie in the sky labels Mar 24, 2021
@dreeves dreeves changed the title Skatesum uses the final road rate, which is sometimes totally wrong Skatesum uses the final redline rate, which is sometimes totally wrong Jul 12, 2021
@dreeves dreeves changed the title Skatesum uses the final redline rate, which is sometimes totally wrong Skatesum uses final redline rate, which is sometimes totally wrong Oct 22, 2021
@dreeves dreeves changed the title Skatesum uses final redline rate, which is sometimes totally wrong Skatesum uses final rate of graph matrix, which is sometimes totally wrong Aug 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant