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

Remove redundant interpolate method for matrixes #49

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

ctessum
Copy link
Contributor

@ctessum ctessum commented Jul 11, 2024

fixes #48

@zsunberg
Copy link
Member

@ctessum This is redundant with #47 and can be closed, right?

@zsunberg zsunberg closed this Jul 11, 2024
@ctessum
Copy link
Contributor Author

ctessum commented Jul 12, 2024

I don't think it's redundant.

The function in question is still there: https://github.com/sisl/GridInterpolations.jl/blob/master/src/GridInterpolations.jl#L142

The original function makes a copy of the matrix as Float64s, and #47 changes that to make a copy of the matrix in the same number type as the original matrix, but what I think is called for is just deleting that function entirely, because I don't think there's any reason to make a copy of that array. (But I could be wrong.)

@zsunberg zsunberg reopened this Jul 12, 2024
@zsunberg
Copy link
Member

oops, sorry! I thought you removed the function in #47, but I didn't look closely.

@zsunberg
Copy link
Member

@himanshugupta1009 you mentioned this to me to day. Just wanted to make sure you saw this. Still would be nice to upgrade DenseArray to AbstractArray in another PR like you mentioned.

@himanshugupta1009
Copy link
Contributor

@zsunberg yes, I will make the change and raise a PR today.

@zsunberg zsunberg merged commit 3149429 into sisl:master Jul 12, 2024
3 checks passed
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.

Redundant and allocating method for matrixes
3 participants