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

Error selection #81

Merged
merged 5 commits into from
Nov 28, 2023
Merged

Error selection #81

merged 5 commits into from
Nov 28, 2023

Conversation

cjrolo
Copy link
Collaborator

@cjrolo cjrolo commented Nov 27, 2023

  • Made Error Function selection decent (via enum!)
  • Fixed a BIG error on polynomial related to error bound compression
  • Made Error function MAE default, then switched to MAPE. MAPE provides the most natural, explainable error function.

/// This function calculates the error between 2 arrays of f64. The results are from 0 to ..
/// Being 0, no error, 1 - 100% error and so on.
/// This uses the provided method to calculte it.
pub fn calculate_error_method(original: &[f64], generated: &[f64], method: ErrorMethod) -> f64 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why use this over just method.error(original, generated)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows to force a specific method if needed. My guess will end as dead code. But I do see some advantages looking ahead. (Mostly for benchmarks).

Copy link
Contributor

@rukai rukai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved to avoid blocking you for a day, but you should resolve the index out of bounds issue before merging.

@cjrolo cjrolo merged commit 7286a87 into main Nov 28, 2023
2 checks passed
@cjrolo cjrolo deleted the error_selection branch November 28, 2023 10:19
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

Successfully merging this pull request may close these issues.

3 participants