Add ekore_capi#537
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new ekore_capi crate to expose ekore’s anomalous dimensions and operator matrix elements through a C ABI, and tightens internal visibility in ekore to keep implementation details private while preserving higher-level APIs.
Changes:
- Added
crates/ekore_capiwith C-ABI entrypoints for unpolarized/polarized anomalous dimensions and unpolarized space-like OMEs, plusCacheallocation/free helpers. - Adjusted visibility in
ekore(manypub→pub(super)/pub(crate), andpub mod→mod) to reduce exposed internals. - Added
cbindgen/cargo-cmetadata and updated the lockfile to include the new crate.
Reviewed changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/ekore/src/operator_matrix_elements/unpolarized/spacelike/as2.rs | Restricts OME helper function visibility to module scope. |
| crates/ekore/src/operator_matrix_elements/unpolarized/spacelike/as1.rs | Restricts OME helper function visibility to module scope. |
| crates/ekore/src/operator_matrix_elements/unpolarized/spacelike.rs | Makes as1/as2 internal modules. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4/gqg.rs | Restricts N3LO component function visibility. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4/gps.rs | Restricts N3LO component function visibility. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4/gnsv.rs | Narrows visibility of gamma_nsv. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4/gnsp.rs | Narrows visibility of gamma_nsp. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4/gnsm.rs | Narrows visibility of gamma_nsm. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4/ggq.rs | Restricts N3LO component function visibility. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4/ggg.rs | Restricts N3LO component function visibility. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as4.rs | Removes public re-exports and makes singlet entry internal. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as3.rs | Restricts NNLO functions to parent module. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as2.rs | Narrows visibility of NLO functions and singlet helper pieces. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as1aem1.rs | Restricts mixed QCD×QED pieces to parent module. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/as1.rs | Narrows visibility of LO helper pieces. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/aem2.rs | Restricts NLO QED pieces to parent module. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike/aem1.rs | Restricts LO QED pieces to parent module. |
| crates/ekore/src/anomalous_dimensions/unpolarized/spacelike.rs | Makes most submodules internal; keeps as1/as2 crate-visible. |
| crates/ekore/src/anomalous_dimensions/polarized/spacelike/as2.rs | Restricts polarized NLO functions to parent module. |
| crates/ekore/src/anomalous_dimensions/polarized/spacelike/as1.rs | Restricts polarized LO functions to parent module. |
| crates/ekore/src/anomalous_dimensions/polarized/spacelike.rs | Makes polarized as1/as2 internal modules. |
| crates/ekore_capi/src/lib.rs | Defines C-facing ComplexF64 and Cache lifecycle API; wires C API modules. |
| crates/ekore_capi/src/ad_us.rs | Adds C ABI wrappers for unpolarized space-like anomalous dimensions (QCD and QCD×QED). |
| crates/ekore_capi/src/ad_ps.rs | Adds C ABI wrappers for polarized space-like anomalous dimensions (QCD). |
| crates/ekore_capi/src/ome_us.rs | Adds C ABI wrappers for unpolarized space-like OME towers. |
| crates/ekore_capi/README.md | Documents crate purpose and citation policy. |
| crates/ekore_capi/cbindgen.toml | Configures C header generation. |
| crates/ekore_capi/Cargo.toml | Declares new crate, crate-type, and cargo-c metadata. |
| Cargo.lock | Adds ekore_capi to the workspace lockfile. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let c = &mut *c; | ||
| let var: [u8; 3] = slice::from_raw_parts(n3lo_variation, 3).try_into().unwrap(); | ||
| let out = slice::from_raw_parts_mut(result, order_qcd); | ||
|
|
||
| for (dst, src) in out | ||
| .iter_mut() | ||
| .zip(spacelike::gamma_ns_qcd(order_qcd, mode, c, nf, var)) | ||
| { | ||
| *dst = src.into(); | ||
| } |
| let c = &mut *c; | ||
| let var: [u8; 4] = slice::from_raw_parts(n3lo_variation, 4).try_into().unwrap(); | ||
| let out = slice::from_raw_parts_mut(result, order_qcd * 4); | ||
|
|
| let c = &mut *c; | ||
| let var: [u8; 3] = slice::from_raw_parts(n3lo_variation, 3).try_into().unwrap(); | ||
| let ncols = order_qed + 1; | ||
| let out = slice::from_raw_parts_mut(result, (order_qcd + 1) * ncols); | ||
|
|
||
| let gamma = spacelike::gamma_ns_qed(order_qcd, order_qed, mode, c, nf, var); | ||
| for (i, row) in gamma.iter().enumerate() { | ||
| for (j, val) in row.iter().enumerate() { | ||
| out[i * ncols + j] = (*val).into(); | ||
| } | ||
| } |
| let c = &mut *c; | ||
| let var: [u8; 7] = slice::from_raw_parts(n3lo_variation, 7).try_into().unwrap(); | ||
| let ncols = order_qed + 1; | ||
| let out = slice::from_raw_parts_mut(result, (order_qcd + 1) * ncols * 16); | ||
|
|
| let c = &mut *c; | ||
| let var: [u8; 3] = slice::from_raw_parts(n3lo_variation, 3).try_into().unwrap(); | ||
| let ncols = order_qed + 1; | ||
| let out = slice::from_raw_parts_mut(result, (order_qcd + 1) * ncols * 4); | ||
|
|
| let c = &mut *c; | ||
| let var: [u8; 4] = slice::from_raw_parts(n3lo_variation, 4).try_into().unwrap(); | ||
| let out = slice::from_raw_parts_mut(result, order_qcd * 4); | ||
|
|
| let c = &mut *c; | ||
| let out = slice::from_raw_parts_mut(result, matching_order_qcd * 9); | ||
|
|
| let c = &mut *c; | ||
| let out = slice::from_raw_parts_mut(result, matching_order_qcd * 4); | ||
|
|
| /// C-compatible representation of a double-precision complex number. | ||
| /// | ||
| /// The memory layout (`re` followed by `im`) matches `num::Complex<f64>` and | ||
| /// the C99 `double complex` / Fortran `COMPLEX(KIND=8)` types. |
|
For change in visibility of ekore functions, I changed all of them to What I will do later:
There are a few doubts for which I want your opinion: For ekore:
For ekore_capi:
cargo cinstall --release -p ekore_capi --destdir=./crates/ekore_capi/dist --prefix=/We can then publish the dist as a GitHub release (as it is done in pineappl).
My initial thought is to make only the Other things I thought we could've done for #518, but can be done here:
Do let me know your thoughts. |
|
CoPilot reviews Summary:
My initial thought is to add safeguards and not bump the Rust version. Please let me know your thoughts too. |
|
Small note, the |
Indeed, I noticed. I hope the relevant people become aware quickly and are able to do something ... |
|
Should I fix #538 here or in a separate branch? |
felixhekhorn
left a comment
There was a problem hiding this comment.
- many comments generalize to other cases
- I think it would be a good idea to split cf61e01 into a separate PR (since it affects lot's of files and does effectively nothing) and then put this one with just the C API on top
|
|
||
| [dependencies] | ||
| ekore = { path = "../ekore", version = "0.0.1" } | ||
| num = "0.4.1" |
There was a problem hiding this comment.
should we write a script (or similar) to enforce the same dependencies in the framework? e.g. all crates use num="0.4.1"
There was a problem hiding this comment.
We can define the version in workspace, i.e. root Cargo.toml, then use that everywhere else.
| use ekore::harmonics::cache::Cache; | ||
| use std::slice; | ||
|
|
||
| /// Compute the tower of non-singlet anomalous dimensions. |
There was a problem hiding this comment.
We need the full documentation here. At first you can try to copy (or guess from other places) and I can revise it afterwards
| order_qcd: usize, | ||
| mode: u16, | ||
| c: *mut Cache, | ||
| nf: u8, | ||
| n3lo_variation: *const u8, |
There was a problem hiding this comment.
If I remember correctly in Rust, unlike in Python, it is good practice to explicitly complain about wrong inputs, right? we are not doing this systematically yet - should we start doing so? How do errors bubble through the C API?
There was a problem hiding this comment.
From what Copilot does, it seems it just wants to leave the result empty - which seems also to be what PineAPPL is doing ...
| c: *mut Cache, | ||
| nf: u8, | ||
| n3lo_variation: *const u8, | ||
| result: *mut ComplexF64, |
There was a problem hiding this comment.
The required length of result must be explicitly mentioned in the docstring. Should we provide a convenience wrapper, e.g. ad_ps_gamma_ns_qcd_len(order_qcd) -> 2*order_qcd (or something similar)? or would it be too much - people should read the docstring and basta?
There was a problem hiding this comment.
It's good to have helper functions, I'll make them.
There was a problem hiding this comment.
I wonder if we should apply some kind of meta programming ... because the function body of ad_ps_gamma_ns_qcd will look the same in all variations. The only thing that might change (and I'm not sure how that would manifest in the interface) is the maximum available order ...
There was a problem hiding this comment.
We can explore this after most of the stuff is done.
all these go into separate PRs; the |
I would say, let's follow our established logic, that we first expose as little as possible. In this case we have to expose
I think this requires some work, because again I want to expose as little as possible. We need to expose the PID as they are an explicit input, but I want to hide all the other things. I suggest to split the files into several parts corresponding to different math/physics and then we only expose what we need.
yes, please. Please import all tests from the Python side which are not implemented yet. My idea is to use these same tests also in the CI to test the capo.
yes, let's follow what other people do (and make our life easier)
yes, we must. If we want to provide an API we must provide the documentation along side it else it is pointless |
We can safely raise the MSRV I would say. Again, let's follow PineAPPL and they are currently at "raised MSRV to 1.91.0
I agree - I think it is the rust way to add guards |
remember that Fortran has very low priority, but all the other stuff is crucial. Now, looking at the flood of comments above I think this PR is a good start, but we should split some stuff from this PR and address problems that got revealed here first, before pushing further on this front (when we will surely discover more problems). In short, let's try to make a bunch of small PRs first instead of one big one |
Let me add a TODO as a comment here: EDIT: moved to top |
A step for #519 ; we explicitly shape our public API along the way #145
In this PR we:
TODO
Create and merge a PR for:
--releasetag. Fix poe compile #541ekorevisibility. Changing visibility of ekore and Splitting constants.rs #543crates/ekore/src/constants.rs#539 Changing visibility of ekore and Splitting constants.rs #543spacelike.rstests.msrv.ymlAdd msrv.yml #545Only after all that, in this PR: