Mock external libome - #562
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The new FFI/build integration has portability and distribution risks (C ABI integer types and C++ build inputs outside the crate) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR introduces a mock external C++ libome (C ABI) and wires it into ekore to provide N3LO (a_s^3) unpolarized, space-like OME entries, then propagates the new supported order and shapes through the C and Python bindings/tests.
Changes:
- Add mock C++
libomewith a C ABI and compile/link it intocrates/ekoreviabuild.rs. - Introduce
as3OME implementation inekorethat callslibomeand extend OME tower support up to N3LO. - Update C API + Python wrappers/tests to accept order
< 4and validate the new N3LO slot.
File summaries
| File | Description |
|---|---|
| extras/gsoc/libome/README.md | Documents the mock C++ library and its dummy-return behavior for FFI testing. |
| extras/gsoc/libome/ome.h | Declares the C ABI surface (complex struct + N3LO OME function prototypes). |
| extras/gsoc/libome/ome.cpp | Implements the mock ABI functions returning dummy complex zeros. |
| crates/ekore/src/operator_matrix_elements/unpolarized/spacelike/as3.rs | New N3LO OME implementation calling the external C ABI. |
| crates/ekore/src/operator_matrix_elements/unpolarized/spacelike.rs | Extends OME tower to include as3 (N3LO) and updates tests accordingly. |
| crates/ekore/Cargo.toml | Adds cc as a build-dependency for compiling the external C++ code. |
| crates/ekore/build.rs | Compiles and links the mock C++ library into ekore. |
| crates/ekore_py/tests/test_ome_us.py | Updates Python tests for new supported order and N3LO zeros. |
| crates/ekore_py/src/ome_us.rs | Updates Python wrapper guard/limits to support order < 4. |
| crates/ekore_capi/tests/c/test_ome_us.c | Updates C tests for new result lengths and N3LO zeros. |
| crates/ekore_capi/src/ome_us.rs | Updates C API guard/limits and result-length helpers for order < 4. |
| Cargo.toml | Adds cc to workspace dependencies. |
| Cargo.lock | Locks cc and related transitive dependencies. |
Review details
Suppressed comments (1)
crates/ekore/src/operator_matrix_elements/unpolarized/spacelike/as3.rs:62
A_nsstill passesnf/etaas u32/i32 even though the C ABI usesunsigned int/int. To keep the ABI correct across platforms, cast tostd::ffi::c_uint/std::ffi::c_intconsistently here as well.
pub(super) fn A_ns(c: &mut Cache, nf: u8, L: f64) -> [[Complex<f64>; 2]; 2] {
let n: OmeComplex = c.n().into();
let nf_u32 = u32::from(nf);
let a_qq_ns = unsafe { Complex::from(ome_as3_AqqNS(n, nf_u32, L, -1)) };
- Files reviewed: 12/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
felixhekhorn
left a comment
There was a problem hiding this comment.
- the "original" depends on GSL (see https://gitlab.com/libome/libome ) - can we add this complication (i.e. depending in addition on another library)?
- this approach is the first and easiest way and the most likely solution we need. For example we could make their repo a submodule here and then adding our interface on top. Can we also interface directly to their C-library, *.so, directly? So we could say please install their library like so (without copying any of their stuff)? And for GSL we must rely on the .so file directly in any case. Actually, now that I write that - we will need their headers always, right?
|
Yes, linking directly to the built If we instead go with the submodule approach, we can compile libome using the cmake crate in build.rs and link GSL like this: Click to expand// Build libome via CMake
let dst = cmake::Config::new("../../extras/libome")
.define("BUILD_SHARED_LIBS", "OFF")
.build();
println!("cargo:rustc-link-search=native={}/lib", dst.display());
println!("cargo:rustc-link-lib=static=ome");
// Link against system GSL
if pkg_config::probe_library("gsl").is_err() {
println!("cargo:rustc-link-lib=dylib=gsl");
println!("cargo:rustc-link-lib=dylib=gslcblas");
println!("cargo:rustc-link-lib=dylib=m");
}And no, Rust does not need .h header files when linking against Unlike C/C++ compilers, rustc reads the function declarations and #[repr(C)] structs directly from our Rust code ( |
|
There is nothing to be added to this PR, right? Then if you agree with my plan of closing the PR, but leave the branch, can you please open a separate PR with a small amendment to |
A step for #519.
In this PR we:
extras/gsoc/libome.build.rs.as3.rs.ekore,ekore_capi, andekore_py.This successfully demonstrates that it is possible to call an external library from the
ekorecrate without hindering performance. If the library usescmakefor build, we can simply change it inCargo.toml.