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

Memory corruption when using custom grid #7

Closed
NotSpecial opened this issue Nov 16, 2018 · 11 comments
Closed

Memory corruption when using custom grid #7

NotSpecial opened this issue Nov 16, 2018 · 11 comments
Assignees

Comments

@NotSpecial
Copy link

NotSpecial commented Nov 16, 2018

Hi!

An first of all, thank you for this great package, both the performance and ease-of-use are amazing.

However, I'm currently encountering an issue that leaves me scratching my head:
Sometimes, using a custom grid results in memory corruption, crashing the interpreter (or the IPython kernel in my case).

As far as I can tell, the the issues lies in the Cython code, as I don't think other parts of the code can cause memory corruption.

Unfortunately, I don't have much experience with Cython to do the debugging myself, and furthermore I could not identify a pattern when the error occurs.

Nevertheless, I have a concrete snippet that crashes every time with the message double free or corruption (out):

import json
import numpy as np
from KDEpy import FFTKDE

with open('data.json') as f:
    data = json.load(f)

kde = FFTKDE().fit(data)

# Evaluating on automatic grid works
kde.evaluate()

# Evaluating on custom grid causes memory corruption
grid = np.linspace(0, 677, 678)
density = kde.evaluate(grid)

And here's the data file: data.zip

(GitHub would not let me upload it without zipping it first)

If you need any more information, please let me know.

Thank you very much in advance!

@NotSpecial
Copy link
Author

Further support that the issues lies within Cython:

If I turn off Cython support by setting:

KDEpy.binning._use_Cython = False

The code above does not crash anymore.

@tommyod
Copy link
Owner

tommyod commented Nov 17, 2018

Hi @NotSpecial , thanks for reporting this! Quick answer since I'm on my phone. This is likely due to data being outside of the grid. E.g. if you use linspace from 2 to 8, and a data point has value 9. Obviously the software should either just warn about the boundary bias, or raise an error - it should not crash. I hope this has not frustrated you too much, and i will fix it when i get the chance.

Could you check if this is the case? Leaviing the issue open until i fix it.

@NotSpecial
Copy link
Author

Hi, and thanks for your response, I finally had time to investigate further now.

You are right, the issue seems to be data points outside the grid. If I select a grid covering the whole range of data (e.g. np.linspace(-705, 705) for the example above), it does not crash anymore.

@tommyod
Copy link
Owner

tommyod commented Nov 19, 2018

@NotSpecial great. I'll leave this open and fix the issue. Currently, I would recommend that you filter out some data prior to running FFTKDE, and then filter the rest afterwards. I haven't run the following code, but I hope it demonstrates what I mean.

# Wish to examine data between 0 and 2.
# First, we use a coarse grid, as this reduces potential boundary bias.
data = np.array(data)
data_filtered = data[(data > -2 ) & ( data < 4)] 

# Prepare the grid
grid = np.linspace(-2, 4, num=2**10)
y = FFTKDE(bw=0.5).fit(data).evaluate()

# Filter the estimate
mask = (grid > 0) & ( grid < 2)
y = y[mask] 
grid = grid[mask] 

plt.plot(grid, y)
``

@NotSpecial
Copy link
Author

Thanks for your response, this will work for me.

Knowing that the issues lies with data outside the grid alone helps me enough to avoid the problem. :)

Thank you as well for working on a fix!

@tommyod
Copy link
Owner

tommyod commented Nov 23, 2018

Should be fixed now. Closing this Issue. Thanks again @NotSpecial .

@tommyod tommyod closed this as completed Nov 23, 2018
@NotSpecial
Copy link
Author

Thank you! :)

@NotSpecial
Copy link
Author

NotSpecial commented Dec 11, 2018

Hi,

I have a little followup question, if you have some time:

Here's where you now check whether the data is inside the grid.
Is there a reason why the grid must be strictly bigger than the data points?
E.g. why is max_grid == max_data not allowed?

I was wondering if a <= check (and >= respectively) would also work or if there is
more to it than I know.

Thank you in advance for your time! :)

@tommyod
Copy link
Owner

tommyod commented Dec 12, 2018

Hi again @NotSpecial,

I don't remember at the top of my head. I can check when I get some time :)

@tommyod tommyod reopened this Dec 12, 2018
@tommyod tommyod self-assigned this Jan 16, 2019
@tommyod tommyod closed this as completed Nov 16, 2020
@mghijs
Copy link

mghijs commented Jan 3, 2023

Hi,

I have a little followup question, if you have some time:

Here's where you now check whether the data is inside the grid. Is there a reason why the grid must be strictly bigger than the data points? E.g. why is max_grid == max_data not allowed?

I was wondering if a <= check (and >= respectively) would also work or if there is more to it than I know.

Thank you in advance for your time! :)

Hi again @NotSpecial,

I don't remember at the top of my head. I can check when I get some time :)

Hi there, would it still not be allowed to have max_grid == max_data (and similar for minima) please?
Because this does not allow me to use the custom grid functionality I think.
I would like to retrieve the KDE-value for each value of the data input.

from KDEpy import FFTKDE
data = np.random.rand(100)
kde = FFTKDE(bw='silverman').fit(data).evaluate(np.sort(data))
File ... 155, in FFTKDE.evaluate(self, grid_points)
    153 max_data = np.max(self.data, axis=0)
    154 if not ((min_grid < min_data).all() and (max_grid > max_data).all()):
  >155     raise ValueError("Every data point must be inside of the grid.")
    156 # Step 1 - Obtaining the grid counts
    157 # TODO: Consider moving this to the fitting phase instead
    158 data = linear_binning(self.data, grid_points=self.grid_points, weights=self.weights)

ValueError: Every data point must be inside of the grid.

@tommyod
Copy link
Owner

tommyod commented Jan 3, 2023

Grid info and data are not the same. Let KDEpy set up its own grid, then inspect the grid - you will see its structure. Also, the grid needs to be equidistant. And the finer the grid, the better the performance (not just on the grid points you are interested in, but over all - data is sampled onto the grid, so a very coarse grid "ruins" the data by sampling it coarsely).

I would like to retrieve the KDE-value for each value of the data input.

I would let KDEPy use a default grid, then sample the results onto your data afterwards. See Fast evaluation on a non-equidistant grid in the examples.

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

No branches or pull requests

3 participants