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

Split equistore in a C-API core crate and a Rust wrapper #85

Merged
merged 2 commits into from
Mar 1, 2023

Conversation

Luthaf
Copy link
Member

@Luthaf Luthaf commented Dec 8, 2022

Closes #102
Closes #140

This is phase I of making equistore compatible with TorchScript, by having a separate C API we can load the code from a single shared library regardless if we are using it from Python/Torch/rascaline/…

  • split equistore crate into a core, containing the C API and a Rust wrapper API
  • the core no longer depends on the ndarray crate
    • loading files now takes a callback to create arrays in which the data will
      be placed
    • serialization is no longer optional, there is a single way the core can be
      built (it had multiple optional feature before)
  • Python bindings explicitly manage the lifetime of array using Py_IncRef and
    Py_DecRef
    This is now Make Tensormap immutable #86
    • we no longer have to explicitly keep references to arrays around, this
      simplifies the code quite a bit

Still TODO:

  • update the CI for the new configuration
  • make sure all tests have been updated, and none is left commented out
  • integrate with rascaline
    • check that the performance hit of not inlining equistore functions is not too bad
  • write API documentation
  • write something on the code organization for new developers
  • remove all TODO in the code
  • add soname to the shared library

📚 Documentation preview 📚: https://equistore--85.org.readthedocs.build/en/85/

@github-actions

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Dec 8, 2022

Here is a pre-built version of the code in this pull request: wheels.zip, you can install it locally by unzipping wheels.zip and using pip to install the file matching your system

@Luthaf Luthaf force-pushed the split-c-api branch 2 times, most recently from 4d52408 to 546123c Compare February 28, 2023 16:05
@Luthaf Luthaf merged commit a554ce4 into master Mar 1, 2023
@Luthaf Luthaf deleted the split-c-api branch March 1, 2023 15:06
@Luthaf
Copy link
Member Author

Luthaf commented Mar 1, 2023

And merged! 🚀

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.

Reusing TensorBlock for creating a TensorMap causes segmentation fault equistore.__version__
2 participants