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

Biweight midcovariance as a CovarianceEstimator #80

Closed
tpgillam opened this issue Nov 17, 2021 · 3 comments · Fixed by #81
Closed

Biweight midcovariance as a CovarianceEstimator #80

tpgillam opened this issue Nov 17, 2021 · 3 comments · Fixed by #81

Comments

@tpgillam
Copy link
Contributor

Hello! We've written an implementation of biweight midcovariance — would you be happy to receive a PR to integrate it into CovarianceEstimation? If so I'll start work on it.

This issue seems related, and proposes adding them to StatsBase: JuliaStats/StatsBase.jl#569 . But to me it seems like it would be better to put a fairly specialised method like this in this package, rather than StatsBase?

@tlienart
Copy link
Collaborator

would you be happy to receive a PR to integrate it

Yes; please kindly also provide a reference implementation as well if there is one or tests with respect to a reference implementation (e.g. if there's a credible R package that does this).

Ideally the PR for a new method should eventually contain:

  • the actual method
  • clear comments in the method clarifying what is done + the computational complexity of the method
  • an update to the docs clarifying what the method does / how it works + links to the relevant literature
  • tests to ensure the method works
  • tests to compare the method to a reference implementation if there is one somewhere

Thanks!

PS: you can also open a draft PR and we can try to help you push it along
PPS: with respect to reference implementation; if there is one but in a proprietary software like Matlab; what you can do is have a few example cases that you evaluate with both this package and Matlab and ensure that the results are comparable (this is what is done in our tests for some of the reference implementation).

@tpgillam
Copy link
Contributor Author

Excellent, thank you for the pointers.

The reference implementations I'm aware of are:

I've never used dataplot before, so my preference would be to use astropy for computing the references, if that suits.

@tlienart
Copy link
Collaborator

astropy is great and it's BSD3 so no license issues 👍

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 a pull request may close this issue.

2 participants