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

Replace interp2d in ProcessedVariable #2670

Closed
rtimms opened this issue Feb 8, 2023 · 11 comments · Fixed by #2865
Closed

Replace interp2d in ProcessedVariable #2670

rtimms opened this issue Feb 8, 2023 · 11 comments · Fixed by #2865
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours

Comments

@rtimms
Copy link
Contributor

rtimms commented Feb 8, 2023

We currently use interp2d when creating processed variables, but this is deprecated. We should switch to RegularGridInterpolator.

DeprecationWarning: `interp2d` is deprecated!
`interp2d` is deprecated in SciPy 1.10 and will be removed in SciPy 1.12.0.

For legacy code, nearly bug-for-bug compatible replacements are
`RectBivariateSpline` on regular grids, and `bisplrep`/`bisplev` for
scattered 2D data.

In new code, for regular grids use `RegularGridInterpolator` instead.
For scattered data, prefer `LinearNDInterpolator` or
`CloughTocher2DInterpolator`.
@rtimms rtimms added the difficulty: easy A good issue for someone new. Can be done in a few hours label Feb 8, 2023
@jeromtom
Copy link
Contributor

  1. Alternative to interp2d: regular grid in the transition guide is relevant to this.

@jaskiratsingh2000
Copy link

HI @rtimms, and @jeromtom Can you let me know that where can I find the processed variable implementation fo "interp2d"? Thanks! :)

@rtimms
Copy link
Contributor Author

rtimms commented Feb 19, 2023

Here. There may be other places where it appears, so worth searching (e.g. doing ctrl+shift+f in vs code).

@jeromtom
Copy link
Contributor

It can be found here pybamm\solvers\processed_variable.py.

@jeromtom
Copy link
Contributor

Using findstr and running findstr /s /i interp2d *.* gives this
image

8 mentions of interp2d, the first 4 in the documentation, and the last 4 in the code.

@valentinsulzer
Copy link
Member

Sites in sites-packages are not part of documentation, they are installed dependencies.

I don't know what's in your build folder but it looks like it's a duplicate of the repository.

So ignore the first 6, it's only the last two

@jaskiratsingh2000
Copy link

@rtimms , @jeromtom , and @tinosulzer Thanks for letting me know, I have made the changes as a part of pull request abiove. Could you please review it once? Thanks in Advance!

@arjxn-py
Copy link
Member

arjxn-py commented Apr 2, 2023

@rtimms If noone is working on this issue, I'd like to give it a try.

@rtimms
Copy link
Contributor Author

rtimms commented Apr 3, 2023

Hi @arjxn-py it looks like @jaskiratsingh2000 is no longer working on this, so feel free to pick it up. There is an open PR #2709 that may help. Thanks!

@arjxn-py
Copy link
Member

Thanks @tinosulzer, I was really stuck at this.

@valentinsulzer
Copy link
Member

Yeah it was getting a bit messy with all the different interpolants, switching to xarray made it much easier

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours
Projects
None yet
5 participants