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

Add a correct way to input a numpy array #55

Merged
merged 6 commits into from
Mar 14, 2024

Conversation

rjanvier
Copy link
Contributor

@rjanvier rjanvier commented Feb 4, 2024

Hi,
Numpy array input was far from being optimal.
he plumbery with SWIG was previously not correctly put in place. As a result, CSF fallback to the void CSF::setPointCloud(std::vector<std::vector<float> > points) that pass the numpy array by value and thus makes two a full copies of the numpy array (and convert double coordinates to float).

This commit correct this behavior by providing correct SWIG bindings for numpy arrays.

@rjanvier
Copy link
Contributor Author

rjanvier commented Feb 4, 2024

PS once accepted and merged, could you please tag the commit on the main branch in order to release the wheels on PyPI?
thanks in advance.

@rjanvier rjanvier mentioned this pull request Feb 7, 2024
@jvavrek
Copy link
Contributor

jvavrek commented Mar 13, 2024

I would highly appreciate this feature as well!

@rjanvier
Copy link
Contributor Author

@jvavrek please look at this fork https://pypi.org/project/CSF-3DFin/

@jianboqi jianboqi merged commit 449c7ba into jianboqi:master Mar 14, 2024
4 checks passed
@jianboqi
Copy link
Owner

Thanks for the comtribution. I have merged the code, and new tag has been created.

@jvavrek
Copy link
Contributor

jvavrek commented Mar 14, 2024

This is great! For me, it turned a particularly large calculation from 5.5 minutes to 1.5 minutes.

@rjanvier rjanvier deleted the rjanvier_numpy_compat branch March 15, 2024 07:20
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.

3 participants