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

remove calc_error_bio in cfeature #324

Merged
merged 2 commits into from
Oct 19, 2023
Merged

remove calc_error_bio in cfeature #324

merged 2 commits into from
Oct 19, 2023

Conversation

anilbey
Copy link
Contributor

@anilbey anilbey commented Oct 17, 2023

This code must have been used in debugging a long time ago. Otherwise return 250.; can't be really the general value that would fit all usages.

  // calculation of GA errors
  template<typename T>
  double calc_error_bio(const vector<T>& v, double bio_mean, double bio_sd)
  {
    if (v.size() != 0) {
      double error = 0.;
      for (size_t i = 0; i < v.size(); i++) {
        error += fabs(v[i] - bio_mean);
      }
      return error / bio_sd / v.size();
    } else {
      return 250.;
    }
  }

@ilkilic
Copy link
Collaborator

ilkilic commented Oct 17, 2023

Yes, it appears that the value 250 has been used as an error code in different parts of the code, as suggested by:

Distance returned when error, default is 250

@anilbey
Copy link
Contributor Author

anilbey commented Oct 17, 2023

Oh I see. So 250.0 is the error code. Thanks

@AurelienJaquier
Copy link
Collaborator

In our use cases, we usually consider a score of 250 to be the worst possible. When something goes wrong with a feature, or if a feature has a score higher than 250, we usually just return 250.

@anilbey
Copy link
Contributor Author

anilbey commented Oct 17, 2023

Thanks, I didn't know it.

This function however is not used anywhere in eFEL. As far as I can see it is computing some cumulative error for a vector.
It is not in the Python API either. Do we still need this function nowadays? Can't we get the same error via numpy (without having to pass the mean and std to C++)?

@AurelienJaquier
Copy link
Collaborator

Yes indeed. I suppose now this score calculation is done by BluePyEfe, and should not be done by efel.

@codecov-commenter
Copy link

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (8ce1476) 57.39% compared to head (5349862) 57.39%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #324   +/-   ##
=======================================
  Coverage   57.39%   57.39%           
=======================================
  Files          29       29           
  Lines        7820     7820           
  Branches     3235     3235           
=======================================
  Hits         4488     4488           
  Misses        847      847           
  Partials     2485     2485           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anilbey anilbey merged commit 782cccc into master Oct 19, 2023
6 checks passed
@anilbey anilbey deleted the old-fn branch October 19, 2023 09:26
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.

4 participants