Skip to content

Use np.interp instead of interpolation.interp #387

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

Merged
merged 3 commits into from
Mar 11, 2024
Merged

Use np.interp instead of interpolation.interp #387

merged 3 commits into from
Mar 11, 2024

Conversation

kp992
Copy link
Contributor

@kp992 kp992 commented Mar 6, 2024

This PR replaces interpolation.interp to np.interp now that numba supports np.interp directly. Also, removes plt.rcParams["figure.figsize"] = (11, 5) #set default figure size redundant line.

@kp992 kp992 requested review from jstac and mmcky March 6, 2024 11:29
@kp992
Copy link
Contributor Author

kp992 commented Mar 6, 2024

@jstac @mmcky I am trying to replace interp function in this PR. There are a few more files which needs this change and will do that in new PR to keep this PR short and easy to review and verify. Thank you!

Copy link

github-actions bot commented Mar 6, 2024

@jstac
Copy link
Contributor

jstac commented Mar 6, 2024

Many thanks @kp992 .

@mmcky or @HumphreyYang , could you please do a first round review of this?

The aim is that swapping to NumPy's interp should not change outputs or degrade performance.

@jstac
Copy link
Contributor

jstac commented Mar 6, 2024

Note that this is related to QuantEcon/lecture-python-advanced.myst#156

Copy link
Contributor

@mmcky mmcky left a comment

Choose a reason for hiding this comment

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

thanks @kp992 I confirm the np.interp has been updated nicely and the function arguments have been switched successfully.

@mmcky
Copy link
Contributor

mmcky commented Mar 6, 2024

@jstac re: performance

lecture current time new time
cake_eating_numerical 30.4 33.8
coleman_policy_iter 24.58 21.52
egm_policy_iter 12.79 6.89
ifp 40.88 51.77
ifp_advanced 32.49 30.85

Some lectures actually improved and some are a bit slower. I don't see any major timing issues.

@mmcky
Copy link
Contributor

mmcky commented Mar 6, 2024

Remaining lectures to migrate

  • lectures/jv.md:from interpolation import interp
  • lectures/mccall_correlated.md:from interpolation import interp
  • lectures/mccall_fitted_vfi.md:from interpolation import interp
  • lectures/navy_captain.md:from interpolation import interp
  • lectures/odu.md:from interpolation import mlinterp, interp
  • lectures/optgrowth_fast.md:from interpolation import interp
  • lectures/wald_friedman.md:from interpolation import interp

@kp992 can you let me know if there is an equivalent to mlinterp in numpy that is compatible with numba

@mmcky
Copy link
Contributor

mmcky commented Mar 6, 2024

@kp992 I know it violates our usual practice but let's try and fix them all in one PR -- so they are in one commit. This lecture series is not in high frequency editing at the moment so I am happy with that approach.

from numba import njit, prange, vectorize
from interpolation import mlinterp, interp
from interpolation import mlinterp
Copy link
Contributor

Choose a reason for hiding this comment

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

@kp992 is there a corresponding function in numpy for mlinterp?

Copy link
Contributor

Choose a reason for hiding this comment

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

No @mmcky, there's not.

There is in SciPy but it's not jit compilable. Let's stick to interpolation for the multivariate case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right, @jstac. I didn't find mlinterp in numpy too.

Copy link
Contributor

@mmcky mmcky Mar 11, 2024

Choose a reason for hiding this comment

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

@kp992 we will need interpolation package updated unless mlinterp doesn't use generated_jit. Would you have time to look into that?

@mmcky
Copy link
Contributor

mmcky commented Mar 8, 2024

thanks @kp992.

@jstac I think this is looking good -- and I have reviewed the function calls. @kp992 has done a nice job.

@mmcky mmcky self-requested a review March 8, 2024 00:42
@@ -1,5 +1,4 @@
import numpy as np
from interpolation import interp
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't see pip install interpolation removed from this lecture... was that task forgotten?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @jstac. I think we can remove this file completely. This isn't use anywhere in any lectures. In optgrowth, it seems to be have been replace by from scipy.interpolate import interp1d.

% git grep bellman_operator
lectures/_static/lecture_specific/odu/odu.py:    def bellman_operator(self, v):
lectures/_static/lecture_specific/wald_friedman/wald_class.py:    def bellman_operator(self, J):
lectures/_static/lecture_specific/wald_friedman/wald_class.py:        J = qe.compute_fixed_point(self.bellman_operator, np.zeros(self.m),
lectures/_static/lecture_specific/wald_friedman/wf_first_pass.py:def bellman_operator(pgrid, c, f0, f1, L0, L1, J):
lectures/_static/lecture_specific/wald_friedman/wf_first_pass.py:bell_op = lambda vf: bellman_operator(pg, 0.5, f0, f1, 5.0, 5.0, vf)

@jstac
Copy link
Contributor

jstac commented Mar 8, 2024

Thanks @mmcky and great work @kp992 .

Please see minor comments above.

@mmcky
Copy link
Contributor

mmcky commented Mar 8, 2024

thanks @jstac for searching the py includes. I had focused on the md 👍

@mmcky
Copy link
Contributor

mmcky commented Mar 11, 2024

@kp992 are you happy for this to be merged?

@jstac jstac merged commit 407b5fd into main Mar 11, 2024
@jstac jstac deleted the interp_fixes branch March 11, 2024 06:25
@kp992
Copy link
Contributor Author

kp992 commented Mar 11, 2024

Thanks @mmcky and @jstac

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