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

Rust moments #46

Merged
merged 24 commits into from
Sep 6, 2024
Merged

Rust moments #46

merged 24 commits into from
Sep 6, 2024

Conversation

arthur-lin1027
Copy link
Collaborator

@arthur-lin1027 arthur-lin1027 commented Sep 2, 2024

An updated version of @YCC-ProjBackups's PR. The code has changed significantly since last year so it's annoying to rebase.

  • I've added code that builds the rust source code in a single pip install .
    • Removed some relic setup stuff (deleted setup.py and setup.cfg) in favor of a single pyproject.toml file that has been properly configured to compile rust code.
    • This improves upon Yong's version where the use of relative imports made building the FFI difficult.
    • I've also added some timing tests that indeed show the rust port to be much quicker, anywhere between 30-150x faster.

One issue is still with the hand-created determinant code, which fails to reproduce the numpy linear algebra code. The workaround isn't too straightforward -- this is either a skill-issue for me, or numpy-rust being immature. I wasn't able to get numpy-rust to interface with nalgebra, the main rust linear algebra package.

  • Supposedly the function PyReadonlyArray::try_as_matrix exists to convert types from numpy-rust to nalgebra, but I couldn't get this to work.
  • For now, the determinants only diverge for near-singular matrices. We calculate determinants of our dilation matrices, which for standard use cases should never be close to singular.
    • The dilation matrix should have a determinant that is almost equal to the inverse product of our semiaxes lengths (for GTOs, the determinant is distorted a bit by the GTO width)

rust/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@rosecers rosecers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still some things that need to be cleaned up, and I'd suggest deleting the rust folder if the file inside it is no-longer necessary. That was causing some confusion.

But overall looks great!

@arthur-lin1027 arthur-lin1027 merged commit 3e6080f into main Sep 6, 2024
4 checks passed
@arthur-lin1027 arthur-lin1027 deleted the rust-moments branch September 6, 2024 21:47
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.

2 participants