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

Additional Features for CubicHermiteInterp #1020

Merged
merged 7 commits into from
Feb 10, 2025

Conversation

alanlujan91
Copy link
Member

This PR adds new methods to CubicHermiteInterp.

Because CubicHermiteInterp uses scipy's CubicHermiteSpline, we can easily port additional methods that could be useful in HARK.

These were initially in #1011 but were moved to a standalone PR for further discussion.

These new methods are:

  1. der_interp(self[, nu]) Construct a new piecewise polynomial representing the derivative.
  2. antider_interp(self[, nu]) Construct a new piecewise polynomial representing the antiderivative.
  3. integrate(self, a, b[, extrapolate]) Compute a definite integral over a piecewise polynomial.
  4. roots(self[, discontinuity, extrapolate]) Find real roots of the the piecewise polynomial.
  5. solve(self[, y, discontinuity, extrapolate]) Find real solutions of the the equation pp(x) == y.

See notebook for more details: https://github.com/alanlujan91/HARK/blob/CHI_new_feats/examples/Interpolation/CubicInterp.ipynb

add new features to CubicHermiteInterp taking advantage of scipy internals
@codecov-commenter
Copy link

codecov-commenter commented Jun 18, 2021

Codecov Report

Merging #1020 (2f05061) into master (d59269d) will decrease coverage by 0.02%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1020      +/-   ##
==========================================
- Coverage   72.53%   72.51%   -0.03%     
==========================================
  Files          68       68              
  Lines       10298    10368      +70     
==========================================
+ Hits         7470     7518      +48     
- Misses       2828     2850      +22     
Impacted Files Coverage Δ
HARK/interpolation.py 42.57% <65.78%> (+1.04%) ⬆️
HARK/ConsumptionSaving/ConsIndShockModel.py 85.86% <100.00%> (+0.01%) ⬆️
HARK/tests/test_interpolation.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d59269d...2f05061. Read the comment docs.

@sbenthall
Copy link
Contributor

@alanlujan91 can you fix the merge conflict? (sorry for duplicating my comment on #1011 )

@mnwhite would you be able to review this?

@mnwhite
Copy link
Contributor

mnwhite commented Feb 10, 2025

I'm working on this now. GitHub flagged a bunch of things as "conflicts" that plainly were not. And it also got upset about notebook execution metadata, which is annoying. It's quite likely that I broke stuff in that commit.

@mnwhite
Copy link
Contributor

mnwhite commented Feb 10, 2025

This has one failing test, and I'm actually not sure whether it should pass. It looks like the test is for whether new cubic interpolator can accepts inputs of shape (N,1) rather than just (N,)-- flat 2D arrays. Is that intended?

@mnwhite
Copy link
Contributor

mnwhite commented Feb 10, 2025

The problem was some selfs that were removed. The init code does a "flat check", but its results weren't being used.

The "flatten check" wasn't actually being used properly. Should be fixed now.
New cubic interpolator wants x to be strictly increasing, but KinkedR gridpoints are weakly increasing. Made tiny change to fix.
@mnwhite mnwhite merged commit 2d76243 into econ-ark:master Feb 10, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants