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

Marathon not correct #1

Open
brianvoe opened this issue Feb 22, 2016 · 13 comments
Open

Marathon not correct #1

brianvoe opened this issue Feb 22, 2016 · 13 comments

Comments

@brianvoe
Copy link

Were you ever able to fix this issue? You have a todo that says this calculation is not correct.

/**
 * Calculate marathon pace paces based on VDOT
 *
 * @todo this calculation is not correct
 *
 * @return integer
 */
function marathon()
{
    return round((($this->easy() - $this->tempo()) * 2/3) + $this->tempo(), 2); // 2/3 from Easy to Tempo.
}
@lsolesen
Copy link
Owner

I do not quite remember what the problem was, as the test here
https://github.com/lsolesen/running-calculations/blob/master/tests/Running/PaceTest.php#L18
seems to pass. Maybe it is a forgotten @todo. I will investigate soon.

The code referenced is from here:
https://github.com/lsolesen/running-calculations/blob/master/src/Running/Vdot/Pace.php#L49

@lsolesen
Copy link
Owner

It seems strange though, that only that function is not wrapped in
this->km()

@brianvoe
Copy link
Author

So if you run marathon off a given vdot. the output number doesnt come close to the jack daniels vdot calculator. https://runsmartproject.com/calculator/

Cause i also though "oh maybe he just forgot to remove the todo"

@lsolesen
Copy link
Owner

I am pretty sure that all the tests for the Paces needs updating. Pull requests welcome :)

@brianvoe
Copy link
Author

I would have not problem updating the paces. I just dont know where to get the formula from.

@lsolesen
Copy link
Owner

If you improve the tests, I will help figure it out.

@brianvoe
Copy link
Author

In this instance tests are somewhat arbitrary considering jack daniels calculator gives a range. Im satisfied as long it is as close as possible. The only one that isnt close is that marathon.

@lsolesen
Copy link
Owner

oki. what should the value be?

lsolesen added a commit that referenced this issue Feb 25, 2016
@brianvoe
Copy link
Author

did you get a change to look at this website? https://runsmartproject.com/calculator/

@lsolesen
Copy link
Owner

lsolesen commented Mar 1, 2016

Yeah, I use it all the time. :)

@brianvoe
Copy link
Author

brianvoe commented Mar 1, 2016

ok if you do a marathon comparison between that site and an output from your class you can see that its off by a good amount.

@lsolesen
Copy link
Owner

Actually it seems that all of the values are off. Pretty sure I just based the tests off the results of the methods from the assumption that they were calculating correctly.

@lsolesen
Copy link
Owner

@brianvoe Could you please check whether this branch #3 has the correct calculation. It seems that marathon() was messed up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants