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

Duplicate of mean_max() algorithm #9

Closed
sladkovm opened this issue Mar 10, 2018 · 5 comments
Closed

Duplicate of mean_max() algorithm #9

sladkovm opened this issue Mar 10, 2018 · 5 comments
Assignees

Comments

@sladkovm
Copy link
Contributor

There are currently two versions:

algorithms.main.mean_max_power() - relies on rolling mean calculation
algorithms.metrics.power_duration_curve() - relies on calculating diff of accumulated energy and is x4 faster

Proposal:

  1. Rename algorithm to mean_max() because it can be applied not only to power but to pace as well
  2. Keep faster implementation with data type casting resolved according to the outcome of Discussion: input and output argument type casting #5
  3. Move it to main, which will serve as a module for the general purpose calculations or according to the outcome of Discussion: project stucture #7
@sladkovm sladkovm self-assigned this Mar 10, 2018
@sladkovm sladkovm mentioned this issue Mar 10, 2018
@sladkovm sladkovm changed the title One version of mean_max() algorithm Duplicate of mean_max() algorithm Mar 10, 2018
@AartGoossens
Copy link
Contributor

  1. Agree. Then this method would live in metrics.core as well?
  2. Totally agree :)
  3. Agree. Although I don't like the name "main" I gave it. Do you have a better idea? And do we even want to keep main when we're moving to separate heartrate/power/speed modules?

@sladkovm
Copy link
Contributor Author

The mean_max() resides in the core and in the metrics.power we would even have a liberty to call a power_duration_curve().

I propose to transfer the content of main to the core or to the specific metrics module depending on what it does.

@AartGoossens
Copy link
Contributor

👍

@sladkovm
Copy link
Contributor Author

Shall I pick up this issue and implement the project structure agreed in #7 ?

@AartGoossens
Copy link
Contributor

Yes if you would like to :)

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