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

scipy isotonic regression #636

Open
nicholasloveday opened this issue Aug 15, 2024 · 2 comments
Open

scipy isotonic regression #636

nicholasloveday opened this issue Aug 15, 2024 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@nicholasloveday
Copy link
Collaborator

Currently sklearn is only used in isotonic regression when functional="mean". This is because the sklearn implementation is about 4 times faster than implementing it ourselves.

Recently scipy added an isotonic regression function https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.isotonic_regression.html .

We should investigate if we can use that and drop the sklearn dependency

@nicholasloveday nicholasloveday added the enhancement New feature or request label Aug 15, 2024
@tennlee tennlee added the good first issue Good for newcomers label Sep 11, 2024
@tennlee
Copy link
Collaborator

tennlee commented Sep 11, 2024

For anyone wanting to do this, there should be plenty of test coverage in scores already. The work would include:

  • Checking whether the scipy implementation has the same functionality as that of sklearn
  • Checking whether the scipy implementation is as fast/efficient as that of sklearn
  • Change the functional code in scores to use the new scipy method
  • Check the unit tests all pass, both with and without dask being installed in the environment
  • If successful, remove sklearn from the dependencies in pyproject.toml

@nicholasloveday
Copy link
Collaborator Author

Additionally, the current implementation uses scipy's interpolate.interp1d which is a legacy function that won't be supported in the future. We should explore replacements

https://docs.scipy.org/doc/scipy/tutorial/interpolate/1D.html#legacy-interface-for-1-d-interpolation-interp1d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants