added econforge interpolation into LinearInterp and scipy interpolati…

[gms158]

…on into CubicInterp

Please ensure your pull request adheres to the following guidelines:

• Tests for new functionality/models or Tests to reproduce the bug-fix in code.
• Updated documentation of features that add new functionality.
• Update CHANGELOG.md with major/minor changes.

[gms158]

I temporarily removed scipy CubicInterp, since it does have minor tolerance value changes, which causes errors in the unit test. I tested scipy CubicHermiteSpline, which resulted in larger tolerance issues than CubicInterp. I removed this to at least get the base LinearInterp changes implemented. I added a try catch around the econforge linear interpolation test, since this will capture the error and should not increase runtimes. If this approach is not desirable, we can try and think of cleaner ways to do this.

[AMonninger]

I'm interested in this and would be happy to review it after @alanlujan91 is done.

[alanlujan91]

@gms158 because EconForge uses numba and numba is for speeding up array arithmetic, I wonder if the error is simply because the test is using ints and not arrays

https://github.com/econ-ark/HARK/runs/6178262613?check_suite_focus=true#step:5:558

Maybe Bilinear and Trilinear should check the type first and if inputs are ints then they should be cast as arrays?

[gms158]

Alan- yes, I was going to look into this more tomorrow. I may or will follow up with a question or two for you tomorrow. -Gry

…

On Tue, Apr 26, 2022 at 6:51 PM alanlujan91 ***@***.***> wrote: @gms158 <https://github.com/gms158> because EconForge uses numba and numba is for speeding up array arithmetic, I wonder if the error is simply because the test is using ints and not arrays https://github.com/econ-ark/HARK/runs/6178262613?check_suite_focus=true#step:5:558 Maybe Bilinear and Trilinear should check the type first and if inputs are ints then they should be cast as arrays? — Reply to this email directly, view it on GitHub <#1136 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAOLRJ322WCGPXXUJ4KWX6DVHBXQRANCNFSM5TY4DOBQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[albop]

I'm currently offline but if you want I can help review this PR next week Le mer. 27 avr. 2022 à 04:49, gms158 ***@***.***> a écrit :

…

Alan- yes, I was going to look into this more tomorrow. I may or will follow up with a question or two for you tomorrow. -Gry On Tue, Apr 26, 2022 at 6:51 PM alanlujan91 ***@***.***> wrote: > @gms158 <https://github.com/gms158> because EconForge uses numba and > numba is for speeding up array arithmetic, I wonder if the error is simply > because the test is using ints and not arrays > > > https://github.com/econ-ark/HARK/runs/6178262613?check_suite_focus=true#step:5:558 > > Maybe Bilinear and Trilinear should check the type first and if inputs are > ints then they should be cast as arrays? > > — > Reply to this email directly, view it on GitHub > <#1136 (comment)>, or > unsubscribe > < https://github.com/notifications/unsubscribe-auth/AAOLRJ322WCGPXXUJ4KWX6DVHBXQRANCNFSM5TY4DOBQ > > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> > — Reply to this email directly, view it on GitHub <#1136 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACDSKPX4A5R25GYHCW5V3LVHCTLRANCNFSM5TY4DOBQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[gms158]

I updated the interpolation code to account for infrequent and outlier cases where the econ forge library fails when evaluating one point consisting of integers, for python 3.8 and higher.

[alanlujan91]

@albop @AMonninger did you want to review / have any comments on this before we merge it?

[pat749]

yes I have reviewed it's good to merge.

[llorracc]

@alanlujan91, do you know any reason we should not merge this?

[alanlujan91]

@alanlujan91, do you know any reason we should not merge this?

No, I was just giving others a chance to comment but it passes all tests so we can merge it

[sbenthall]

Please add an update to the CHANGELOG describing this feature to the PR

https://github.com/econ-ark/HARK/blob/master/Documentation/CHANGELOG.md

[sbenthall]

This PR needs documentation according to the NumPy / SciPy standard:

https://github.com/econ-ark/HARK/blob/master/Documentation/contributing/CONTRIBUTING.md#guidelines

[sbenthall]

Superceded by #1151