OpenChemistry / OpenChemistry/stempy
Memory issues when adding a new function with pybind11
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 26
- Forks
- 12
- Avg merge
- 1d 2h
- Merged PRs (30d)
- 5
Description
In attempting to add a new function, callable from Python, similar to electron_count, the function gives a segmentation fault on returning an ElectronCountedData object if no py::array_t objects are included as parameters to that function. However, if one or more py::array_t arrays are given as parameters, it seems to work without problems.
The addition of the function follows the general methodology used in: https://github.com/OpenChemistry/stempy/blob/master/python/image.cpp
A simple example is given here: https://github.com/jerenner/stempy/tree/example
(diff is here: https://github.com/jerenner/stempy/commit/60ae39090287c3f083f120e6b0df16c6fdac4cec)
The example runs the following code:
{
Dimensions2D scanDimensions = { 0, 0 };
Dimensions2D frameSize = { 0, 0 };
Events events;
events.resize(10);
ElectronCountedData ret;
ret.data = events;
ret.scanDimensions = scanDimensions;
ret.frameDimensions = frameSize;
return ret;
}
using 2 different methods, one which takes a py::array_t and the other that does not.
ElectronCountedData testMethodBasic(int test)ElectronCountedData testMethodArray(int test, const float test_arr[])
A Jupyter notebook showing the functions run in Python is here: https://github.com/jerenner/4dstem/blob/main/pybind_test.ipynb
For some reason (which could be machine-dependent, as this has only been tested on one machine), the method that takes the array as a parameter does not produce any problems, but the one without any array parameters gives a segmentation fault, which appears to occur on line 28 of https://github.com/OpenChemistry/stempy/blob/master/python/image.cpp:
return py::array(ptr->size(), ptr->data(), capsule);
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with python/image.cpp around the reported line 28 and reproduce the crash using the example repository, commit 60ae390, and pybind_test.ipynb. Compare testMethodBasic with testMethodArray and trace the returned ElectronCountedData and array handling. Done means both functions run from Python without a segmentation fault.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp, python
- Domain
- backend-api-design
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100