experimental: Add minimal rust backend. #405

matte1 · 2024-11-02T23:17:22Z

First pass at adding a simple rust backend that use nalgebra for its matrix library.

symforce/codegen/format_util.py

symforce/test_util/backend_coverage_expressions.py

..._codegen_test_data/symengine/symforce_rust_codegen_test/src/backend_test_function_float32.rs

matte1 · 2024-11-02T23:22:51Z

test/symforce_rust_codegen_test.py

+TEST_DATA_DIR = (
+    path_util.symforce_data_root()
+    / "test"
+    / "symforce_function_codegen_test_data"
+    / symforce.get_symbolic_api()
+    / "symforce_rust_codegen_test"
+    / "src"
+)


I'd like to actually compile the rust code to make sure we are generating valid rust code? One step further would be to write unit tests for the generated rust code. @aaron-skydio could you help me figure out the best path forward on it.

Cool, yeah that seems like a great idea. Some thoughts:

I'm guessing what we want to use to build the rust code is cargo, or rustc, called directly and not managed by cmake. In that case, I'm guessing the easiest way to do this is to write a python unit test which e.g. calls cargo to build the generated code and build a unit test that exercises the generated code, and then invokes the unit test. Unless it's easier to make this a cmake-native test than I'm expecting; it seems like it's probably unnecessary to add rust to our cmake build just for this one test

I think we'll want to run that python test only if a rust toolchain is available, this is similar to what we do for numba and pytorch

For actually running that test in CI, I think we should install a rust toolchain in one or more of the jobs here, and then your python unit test should run in those jobs

Does that all make sense?

Yup! I just added it to this test. For now I think if we just build these generated files thats probably sufficient. I think I added rust to the ci.yml correctly but I'm not sure how to kick off ci?

Running CI on a PR here requires a maintainer approving each run - I can do that (I'll hit the button again now), but of course that doesn't work great for you to iterate on things. If you have GitHub Actions enabled on your fork, you can run the CI job manually on your branch from the Actions tab on your fork, like this:

Ahh thanks. I don't use github all that much. I'm still getting quite a few errors on the cuda codegen because I changed the order in which some expressions get generated. I thought I updated all the necessary files but it still is giving me issues. Couldn't figure out tonight, but hopefully will track it down this weekend.

https://github.com/matte1/symforce/actions/runs/11809269710/job/32899285933

Yup! Your totally right, sorry I thought I got them all. Still working through some more!

Looks like we have passing tests!
https://github.com/matte1/symforce/actions/runs/11839918889/job/32992525216

Yay! I'll try and get this merged soon

Thanks for all the help with this @aaron-skydio !

Let me know if there is anything else I can do to help get this over the line!

avsaase

Amazing work! I took a stab at this a few weeks ago but I quickly gave up after getting stuck with the templates.

I know this is experimental and a work in progress but I couldn't help giving it a quick test. I only did a small test with some matrix multiplications but I ran into a few small issues so I figured I might as well leave some suggestions.

symforce/codegen/backends/rust/templates/util/util.jinja

symforce/codegen/backends/rust/templates/function/FUNCTION.rs.jinja

* Fixed issue with matrix creation * Used fully qualified nalgebra path * Add better support for generating / compiling tests * Added unit test for matrix math.

matte1 · 2024-11-05T03:30:36Z

Thanks for testing / reviewing @avsaase. Took another quick pass at improving it.

aaron-skydio

Awesome! Great work, I'm excited for this. Overall this looks great, mostly had comments on some style things, and tried to answer your questions about testing and such

symforce/codegen/__init__.py

symforce/codegen/backends/rust/README.rst

symforce/codegen/backends/rust/rust_code_printer.py

symforce/test_util/backend_coverage_expressions.py

test/symforce_function_codegen_test_data/symengine/symforce_rust_codegen_test/Cargo.lock

..._codegen_test_data/symengine/symforce_rust_codegen_test/src/backend_test_function_float32.rs

aaron-skydio · 2024-11-07T00:25:09Z

test/symforce_rust_codegen_test.py

+TEST_DATA_DIR = (
+    path_util.symforce_data_root()
+    / "test"
+    / "symforce_function_codegen_test_data"
+    / symforce.get_symbolic_api()
+    / "symforce_rust_codegen_test"
+    / "src"
+)


Cool, yeah that seems like a great idea. Some thoughts:

I'm guessing what we want to use to build the rust code is cargo, or rustc, called directly and not managed by cmake. In that case, I'm guessing the easiest way to do this is to write a python unit test which e.g. calls cargo to build the generated code and build a unit test that exercises the generated code, and then invokes the unit test. Unless it's easier to make this a cmake-native test than I'm expecting; it seems like it's probably unnecessary to add rust to our cmake build just for this one test

I think we'll want to run that python test only if a rust toolchain is available, this is similar to what we do for numba and pytorch

For actually running that test in CI, I think we should install a rust toolchain in one or more of the jobs here, and then your python unit test should run in those jobs

Does that all make sense?

test/symforce_rust_codegen_test.py

aaron-skydio · 2024-11-07T00:32:05Z

There might be some issues with our CI right now; I'm letting it run, but generally I need to fix up some things so there might be some failures that aren't related to this PR

matte1 · 2024-11-11T00:54:10Z

@aaron-skydio Thanks for the review! I did my best to address them and left them open for you to resolve. I think if we are happy with where this landed could you help me kick off CI so that I can get it all passing?

symforce/test_util/backend_coverage_expressions.py

symforce/codegen/backends/rust/rust_config.py

..._codegen_test_data/symengine/symforce_rust_codegen_test/src/backend_test_function_float64.rs

test/symforce_rust_codegen_test.py

experimental: Add minimal rust backend.

8c0b093

First pass at adding a simple rust backend that use nalgebra for its matrix library.

matte1 commented Nov 2, 2024

View reviewed changes

matte1 mentioned this pull request Nov 2, 2024

Rust codegen support #374

Open

avsaase reviewed Nov 4, 2024

View reviewed changes

Address comments

d3ba6d1

* Fixed issue with matrix creation * Used fully qualified nalgebra path * Add better support for generating / compiling tests * Added unit test for matrix math.

aaron-skydio reviewed Nov 7, 2024

View reviewed changes

Address comments

8778a9f

matte1 force-pushed the rust_backend branch from ccebbde to 8778a9f Compare November 11, 2024 00:53

aaron-skydio reviewed Nov 12, 2024

View reviewed changes

symforce/test_util/backend_coverage_expressions.py Outdated Show resolved Hide resolved

symforce/test_util/backend_coverage_expressions.py Outdated Show resolved Hide resolved

aaron-skydio reviewed Nov 12, 2024

View reviewed changes

Address comments

63c5cfc

matte1 force-pushed the rust_backend branch from 0e34fa8 to 63c5cfc Compare November 13, 2024 00:38

Update sympy tests

0d07664

matte1 force-pushed the rust_backend branch from a0d0792 to 0d07664 Compare November 14, 2024 03:54

Fix mypy types

078400b

matte1 force-pushed the rust_backend branch from d314979 to 078400b Compare November 14, 2024 04:52

Run ruff autoformatting

d924ec8

matte1 force-pushed the rust_backend branch 2 times, most recently from e67ade6 to 8726f8b Compare November 14, 2024 14:24

Fix pylint errors

f53cb99

matte1 force-pushed the rust_backend branch from 8726f8b to f53cb99 Compare November 14, 2024 15:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

experimental: Add minimal rust backend. #405

experimental: Add minimal rust backend. #405

matte1 commented Nov 2, 2024

matte1 Nov 2, 2024

aaron-skydio Nov 7, 2024

matte1 Nov 11, 2024

aaron-skydio Nov 12, 2024

matte1 Nov 13, 2024

matte1 Nov 14, 2024

matte1 Nov 14, 2024

aaron-skydio Nov 14, 2024

matte1 Nov 14, 2024

matte1 Nov 21, 2024

avsaase left a comment

matte1 commented Nov 5, 2024

aaron-skydio left a comment

aaron-skydio Nov 7, 2024

aaron-skydio commented Nov 7, 2024

matte1 commented Nov 11, 2024

experimental: Add minimal rust backend. #405

Are you sure you want to change the base?

experimental: Add minimal rust backend. #405

Conversation

matte1 commented Nov 2, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avsaase left a comment

Choose a reason for hiding this comment

matte1 commented Nov 5, 2024

aaron-skydio left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aaron-skydio commented Nov 7, 2024

matte1 commented Nov 11, 2024