Rust wrapper for porgram functionality #771

Koren-Brand · 2025-02-12T11:37:27Z

Rust wrapper for program and symbol added
Fixed extension field missing from backend implementations of program executor

CUDA backend branch (Until it will be merged to main)
cuda-backend-branch: koren/register_program_execution_for_extension_field

Signed-off-by: Koren-Brand <[email protected]>

…for returning value programs Signed-off-by: Koren-Brand <[email protected]>

Signed-off-by: Koren-Brand <[email protected]>

… field functionality Signed-off-by: Koren-Brand <[email protected]>

Signed-off-by: Koren-Brand <[email protected]>

icicle/src/vec_ops.cpp

wrappers/rust/icicle-core/src/program/tests.rs

wrappers/rust/icicle-core/src/program/mod.rs

jeremyfelder · 2025-02-12T12:27:30Z

wrappers/rust/icicle-core/src/vec_ops/mod.rs

@@ -261,6 +275,31 @@ fn check_vec_ops_args_slice<F>(
    setup_config(input, input, output, cfg, batch_size)
 }

+fn check_execute_program<F, Data>( // COMMENT do we need this check in Rust (or any of the other checks)? seems to bloat rust for stuff not even implemented in cpp.


What do you mean by "for stuff not even implemented in cpp"?

these are checks that the cpp backend doesn't do, Miki asked me whether or not do I keep this checks or not

I'm keeping these checks currently to align with the vecops checks in Rust for now

icicle/include/icicle/program/program.h

icicle/include/icicle/program/returning_value_program.h

yshekel · 2025-02-12T15:06:54Z

icicle/src/program/program_c_api.cpp

+  return eIcicleError::SUCCESS;
+}
+
+void CONCAT_EXPAND(FIELD, generate_program)(ProgramHandle program, SymbolHandle* parameters_ptr, int nof_parameters)


why no return value here?

Changed to return eIcicleError (which would return success or invalid argument)

icicle/src/program/program_c_api.cpp

icicle/src/symbol/symbol_api.cpp

yshekel · 2025-02-12T15:12:18Z

icicle/src/symbol/symbol_api.cpp

+extern "C" { // TODO remove program from names
+// Symbol functions
+// Constructors
+SymbolHandle CONCAT_EXPAND(FIELD, create_empty_symbol)() { return new Symbol<scalar_t>(); }


Can you avoid empty symbol and create it as input directly? Why would you have an empty symbol in Rust?

I was just propagating the public constructors from the cpp side. @mickeyasa @idanfr-ingo do you think it's relevant for a rust user?

Changed to a create_input_symbol function instead

yshekel · 2025-02-12T15:12:44Z

icicle/src/symbol/symbol_api.cpp

+  return this_symbol;
+}
+
+SymbolHandle CONCAT_EXPAND(FIELD, add_symbols)(const SymbolHandle op_a, const SymbolHandle op_b)


You don't check for nullptr everywhere in this file. You need to

icicle/src/vec_ops.cpp

wrappers/rust/icicle-core/src/program/mod.rs

wrappers/rust/icicle-core/src/sumcheck/tests.rs

yshekel · 2025-02-27T13:15:03Z

wrappers/rust/icicle-core/src/traits.rs

@@ -39,6 +39,12 @@ pub trait Arithmetic: Sized + Add<Output = Self> + Sub<Output = Self> + Mul<Outp
    fn pow(self, exp: usize) -> Self;
 }

+pub type HandleCPP = *const c_void;


This is a little strange. Why cpp? maybe HandleFFI? it's just a pointer, it's not necessarily c++ and the name is strange

Would you prefer not having the type at all?

yes. Move it to the macros internally like other APIs

yshekel · 2025-02-27T13:17:15Z

wrappers/rust/icicle-core/src/traits.rs

+}
+
+pub trait DeletableHandle: Handle {
+    fn delete_handle(handle: HandleCPP);


I think it should be
fn delete_handle(&mut self);
Otherwise you do something like a.delete(b) to delete b with the object a.

the function is removed

You don't need it then?

nope the deleting happens in the release pool

yshekel · 2025-02-27T13:18:03Z

wrappers/rust/icicle-core/src/symbol/mod.rs

+      impl Symbol<$field> for FieldSymbol {
+        fn new_input(in_idx: u32) -> Result<Self, eIcicleError> {
+          unsafe {
+            let handle = ffi__input_symbol(in_idx);


should it be ffi_input_input_symbol with a single underscore?

It might be a typo I didn't notice

yshekel · 2025-02-27T13:19:56Z

wrappers/rust/icicle-core/src/symbol/mod.rs

+              unsafe {
+                let ffi_status = ffi_delete_symbol(handle);
+                if ffi_status != eIcicleError::Success {
+                  panic!("Couldn't delete symbol, due to {:?}", ffi_status);


why panic? you can return the error instead

Function removed

wrappers/rust/icicle-core/src/vec_ops/mod.rs

yshekel · 2025-02-27T13:23:36Z

wrappers/rust/icicle-core/src/vec_ops/mod.rs

      $field:ident
    ) => {
        pub(crate) mod test_vecops {
            use super::*;
            use icicle_runtime::test_utilities;
            use icicle_runtime::{device::Device, runtime};
            use std::sync::Once;
+            // use crate::symbol::$field_prefix_ident::Symbol;


wrappers/rust/icicle-core/src/vec_ops/mod.rs

Signed-off-by: Koren-Brand <[email protected]>

Signed-off-by: Stanislav Polonsky <[email protected]>

docs/docs/icicle/rust-bindings/program.md

examples/rust/sumcheck/src/main.rs

icicle/include/icicle/program/program.h

docs/docs/icicle/rust-bindings/program.md

idanfr-ingo · 2025-03-02T11:43:46Z

docs/docs/icicle/rust-bindings/program.md

+
+# Usage
+This section will outline how to use Program and Symbol, mirroring the examples from the [cpp overview](../primitives/program.md). The program use-case splits to three steps:
+1. Defining a function/lambda that describes the program to be ran (or choosing one of the predefined list?).


I think you should at list show how to define a program from the pre-defined enum (the same way I did it in the C++ doc).
Also remove the question mark.

docs/docs/icicle/rust-bindings/program.md

wrappers/rust/icicle-curves/icicle-bls12-377/src/program/mod.rs

wrappers/rust/icicle-curves/icicle-bls12-377/src/symbol/mod.rs

wrappers/rust/icicle-curves/icicle-bw6-761/src/program/mod.rs

wrappers/rust/icicle-curves/icicle-bw6-761/src/symbol/mod.rs

mickeyasa

comments inside

yshekel · 2025-03-03T10:22:15Z

docs/docs/icicle/rust-bindings/program.md

@@ -0,0 +1,150 @@
+# Rust FFI Bindings for Program
+
+>**_NOTE:_**


I think this is not the format for docsaurus. Should be like

icicle/docs/docs/icicle/rust-bindings/polynomials.md

Line 3 in 6251c3a

:::note

yshekel · 2025-03-03T10:25:11Z

docs/docs/icicle/rust-bindings/program.md

+## Defining a Function for Program
+A function operating on a vector of symbols, with outputs being written to said input vector. The input symbols in the vector represent inputs and outputs of field elements, and will be replaced by vectors of field elements when executed.
+>**_NOTE:_**
+> The defined function defines arithmetic operations to be done in series, without control-flow i.e. loops, conditions etc.


This not accurate and maybe confusing. You can use control flow but the lambda is traced and the path taken is the computation that is later computed.

I suggest "The defined function performs arithmetic operations in a traced execution, meaning control flow (e.g., loops, conditions) is not parsed. Instead, the computation follows the exact execution path taken during tracing, which determines the final computation that will be performed."

changed to

:::note
The defined function defines arithmetic operations to be done in series, and could be represented as set of equations (for each output). Practically, control flow (e.g., loops, conditions) is not parsed, instead the computation follows the exact execution path taken during tracing, which determines the final computation that will be performed.
:::

icicle/include/icicle/program/program.h

yshekel

You did not resolve a few things. Please do and I will approve

Signed-off-by: Koren-Brand <[email protected]>

Koren-Brand added 7 commits January 26, 2025 14:42

Rust wrapper for symbol

463ebd3

Signed-off-by: Koren-Brand <[email protected]>

A commit for Jeremy to see the API (Though code is still not functional)

4c3e83f

Signed-off-by: Koren-Brand <[email protected]>

Split Symbol and Program to separate files in Rust

e9b51bf

Signed-off-by: Koren-Brand <[email protected]>

Fixed UI issue with reference operator overloading and added wrapper …

35a1a27

…for returning value programs Signed-off-by: Koren-Brand <[email protected]>

Tests for program implemented in vecops

5443aac

Signed-off-by: Koren-Brand <[email protected]>

Merge main into branch to allow testing with updated backend

f487a09

Signed-off-by: Koren-Brand <[email protected]>

Expanded functionality for all fileds in the Rust wrapper + Extension…

871408d

… field functionality Signed-off-by: Koren-Brand <[email protected]>

Koren-Brand requested review from yshekel, jeremyfelder, mickeyasa, LeonHibnik and idanfr-ingo February 12, 2025 11:37

Koren-Brand added 2 commits February 12, 2025 13:39

Spelling error - changed comment

2a4316c

Signed-off-by: Koren-Brand <[email protected]>

formatting

56509b9

Signed-off-by: Koren-Brand <[email protected]>

jeremyfelder reviewed Feb 12, 2025

View reviewed changes

icicle/src/vec_ops.cpp Show resolved Hide resolved

jeremyfelder reviewed Feb 12, 2025

View reviewed changes

wrappers/rust/icicle-core/src/program/tests.rs Outdated Show resolved Hide resolved

jeremyfelder reviewed Feb 12, 2025

View reviewed changes

wrappers/rust/icicle-core/src/program/mod.rs Outdated Show resolved Hide resolved

jeremyfelder reviewed Feb 12, 2025

View reviewed changes

wrappers/rust/icicle-core/src/program/mod.rs Outdated Show resolved Hide resolved

jeremyfelder reviewed Feb 12, 2025

View reviewed changes