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.

Sign up for GitHub

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

Jump to bottom

ENH,BIND: Return more PointClouds #35

Open
RuhiRG opened this issue Jul 2, 2023 · 1 comment · May be fixed by #37
Open

ENH,BIND: Return more PointClouds #35

RuhiRG opened this issue Jul 2, 2023 · 1 comment · May be fixed by #37
Labels
enhancement New feature or request gsoc23 For GSoC23 and related outputs python-bindings For discussions of the python bindings

Comments

@RuhiRG
Copy link
Member

RuhiRG commented Jul 2, 2023

In dSEAMS raw pointers have been used, but in pybind11 when I bind a function with raw pointers. It gives segmentation error. Here is the git commit I made. The error it shows when I try to use ipython is as follows:

In [1]: cyoda.clearPointCloud(mpt)
Out[1]: <bbdir.cyoda.PointCloudDouble at 0x7f72d1e91130>

In [2]: mpt = cyoda.clearPointCloud(mpt)

In [3]: mpt
Out[3]: <bbdir.cyoda.PointCloudDouble at 0x7f72d1c333f0>

In [4]: cyoda.readXYZ("conftest.xyz", mpt)
Stack trace (most recent call last):
#31   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e8aa08d, in _PyRun_SimpleFileObject
#30   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e8aa70f, in 
#29   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e895772, in 
#28   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e899369, in 
#27   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e87a9de, in PyEval_EvalCode
#26   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e87b325, in 
#25   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7c73ce, in _PyEval_EvalFrameDefault
#24   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7efc70, in PyObject_Call
#23   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e806802, in 
#22   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e806a73, in 
#21   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7c385c, in _PyEval_EvalFrameDefault
#20   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7d04cb, in PyObject_Vectorcall
#19   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7ee728, in 
#18   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e88f996, in 
#17   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e88edb8, in 
#16   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7c6478, in _PyEval_EvalFrameDefault
#15   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e88edb8, in 
#14   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7c6478, in _PyEval_EvalFrameDefault
#13   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e88edb8, in 
#12   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7c385c, in _PyEval_EvalFrameDefault
#11   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7d04cb, in PyObject_Vectorcall
#10   Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7d05de, in 
#9    Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e89208d, in 
#8    Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e87a9de, in PyEval_EvalCode
#7    Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e87b325, in 
#6    Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7c385c, in _PyEval_EvalFrameDefault
#5    Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7b6e12, in _PyObject_MakeTpCall
#4    Object "/users/home/ruhila/micromamba/envs/dseams/bin/python3.11", at 0x56400e7d6d45, in 
#3    Object "/users/home/ruhila/Git/Github/dSEAMS/pyseams/bbdir/cyoda.cpython-311-x86_64-linux-gnu.so", at 0x7f72d1be08a7, in 
#2    Object "/users/home/ruhila/Git/Github/dSEAMS/pyseams/bbdir/cyoda.cpython-311-x86_64-linux-gnu.so", at 0x7f72d1be94a3, in 
#1    Object "/users/home/ruhila/micromamba/envs/dseams/lib/libyodaLib.so", at 0x7f72d1aef46c, in sinp::readXYZ(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, molSys::PointCloud<molSys::Point<double>, double>*)
#0    Object "/lib64/libc.so.6", at 0x7f72e1700e0b, in 
Segmentation fault (Address not mapped to object [0x30])
Segmentation fault (core dumped)

The C++ function code takes the input and process the object in place, returning an integer status code. The pybind11 documentation notes that there are issues with changes to native containers. The simplest solution would be to change the seams-core function to take a single input string (filename) and return a populated PointCloud object instead of an int.
@HaoZeke HaoZeke added the gsoc23 For GSoC23 and related outputs label Jul 2, 2023
@HaoZeke
Copy link
Member

HaoZeke commented Jul 2, 2023 •
edited

I agree, @amritagos the function signature is here [rendered doc]:

seams-core/src/seams_input.cpp

Lines 73 to 96 in 13e0104

int sinp::readXYZ(std::string filename,
molSys::PointCloud<molSys::Point<double>, double> *yCloud) {
std::unique_ptr<std::ifstream> xyzFile;
xyzFile = std::make_unique<std::ifstream>(filename);
std::string line; // Current line being read in
std::vector<std::string> tokens; // Vector containing word tokens
std::vector<double> numbers; // Vector containing type double numbers
int nop = -1; // Number of atoms in targetFrame
int iatom = 1; // Current atom being filled into the PointCloud
molSys::Point<double> iPoint; // Current point being read in from the file
double xLo, xHi, yLo, yHi, zLo,
zHi; // Box lengths extrapolated from the least and greatest coordinates
if (!(gen::file_exists(filename))) {
std::cout
<< "Fatal Error: The file does not exist or you gave the wrong path.\n";
// Throw exception?
return 1;
}
// --------
// Before filling up the PointCloud, if the vectors are filled
// empty them
*yCloud = molSys::clearPointCloud(yCloud);

It seems like the best way would indeed be to return pointCloud instances. @RuhiRG it would be fantastic to have a PR closing this :)

@HaoZeke HaoZeke added enhancement New feature or request python-bindings For discussions of the python bindings labels Jul 2, 2023
@RuhiRG RuhiRG mentioned this issue Jul 5, 2023
Merged
This was referenced Aug 23, 2023
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request gsoc23 For GSoC23 and related outputs python-bindings For discussions of the python bindings
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

ENH: Remove lua bindings RuhiRG/seams-core
2 participants
@HaoZeke @RuhiRG