imbajin commented on code in PR #359: URL: https://github.com/apache/hugegraph-computer/pull/359#discussion_r3766954502
########## computer-rust/src/ffi/c_api.rs: ########## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You me obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::kernel::csr::CsrGraph; +use crate::kernel::pagerank::PageRankKernel; +use crate::kernel::sssp::SsspKernel; +use crate::RUST_KERNEL_VERSION; +use std::ffi::CString; +use std::os::raw::c_char; +use std::ptr; +use std::slice; + +pub struct GraphBuilder { + num_vertices: u32, + edges: Vec<(u32, u32, f64)>, + csr: Option<CsrGraph>, +} + +#[no_mangle] +pub extern "C" fn computer_graph_create(num_vertices: u32) -> *mut GraphBuilder { + let builder = Box::new(GraphBuilder { + num_vertices, + edges: Vec::new(), + csr: None, + }); + Box::into_raw(builder) +} + +#[no_mangle] +pub extern "C" fn computer_graph_add_edge( + handle: *mut GraphBuilder, + src: u32, + dst: u32, + weight: f64, +) -> i32 { + if handle.is_null() { + return -1; + } + let builder = unsafe { &mut *handle }; + builder.edges.push((src, dst, weight)); Review Comment: ‼️ `computer_graph_add_edge()` still returns success after `computer_graph_finalize()` has populated `builder.csr`. Subsequent edges are appended to `edges`, but both compute functions keep reading the old CSR, so the C caller silently computes an obsolete graph. Please reject additions after finalization or invalidate/rebuild the CSR before allowing computation. ########## computer-rust/src/kernel/pagerank.rs: ########## @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::kernel::csr::CsrGraph; + +#[derive(Debug, Clone)] +pub struct PageRankKernel { + damping_factor: f64, + max_iterations: u32, + tolerance: f64, +} + +impl PageRankKernel { + pub fn new(damping_factor: f64, max_iterations: u32, tolerance: f64) -> Self { Review Comment: ⚠️ `PageRankKernel::new()` accepts non-finite or out-of-range parameters without validation. A `NaN` damping factor produces NaN ranks, and a `NaN` tolerance prevents convergence because every comparison is false; damping values outside `[0, 1]` also violate the probability contract. Please validate finite damping/tolerance at the API boundary and return an error for invalid input. ########## computer-rust/src/kernel/aggregator.rs: ########## @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use std::sync::atomic::{AtomicU64, Ordering}; + +pub struct AtomicAggregator { + sum_bits: AtomicU64, + count: AtomicU64, +} + +impl Default for AtomicAggregator { + fn default() -> Self { + Self::new() + } +} + +impl AtomicAggregator { + pub fn new() -> Self { + Self { + sum_bits: AtomicU64::new(0f64.to_bits()), + count: AtomicU64::new(0), + } + } + + pub fn aggregate(&self, value: f64) { + self.count.fetch_add(1, Ordering::Relaxed); + let mut current_bits = self.sum_bits.load(Ordering::Relaxed); + loop { + let current_val = f64::from_bits(current_bits); + let new_val = current_val + value; + let new_bits = new_val.to_bits(); + + match self.sum_bits.compare_exchange_weak( + current_bits, + new_bits, + Ordering::SeqCst, + Ordering::Relaxed, + ) { + Ok(_) => break, + Err(actual_bits) => current_bits = actual_bits, + } + } + } + + pub fn get_sum(&self) -> f64 { + f64::from_bits(self.sum_bits.load(Ordering::SeqCst)) + } + + pub fn get_count(&self) -> u64 { + self.count.load(Ordering::SeqCst) + } + + pub fn reset(&self) { Review Comment: ⚠️ `reset()` clears `sum_bits` and `count` in separate atomic operations while `aggregate()` updates them separately. A concurrent reset can expose `count == 0` with a non-zero sum, or lose an aggregate between the two stores. Please synchronize reset with aggregation, add a generation/lock, or document and test a quiescent-reset contract. ########## vermeer/apps/compute/rust_bridge.go: ########## @@ -0,0 +1,107 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package compute + +import ( + "fmt" + "math" +) + +// RustKernelBridge manages interaction with high-performance Rust computing kernels. +type RustKernelBridge struct { + available bool + version string +} + +func NewRustKernelBridge() *RustKernelBridge { + return &RustKernelBridge{ + available: false, + version: "1.5.0-go-fallback", + } +} + +func (b *RustKernelBridge) IsAvailable() bool { + return b.available +} + +func (b *RustKernelBridge) Version() string { + return b.version +} + +// ComputePageRank calculates PageRank with fallback to Go execution when native library is inactive. +func (b *RustKernelBridge) ComputePageRank(numVertices uint32, edges [][2]uint32, dampingFactor float64, maxIterations uint32, tolerance float64) ([]float64, error) { + if numVertices == 0 { + return nil, fmt.Errorf("numVertices must be greater than 0") + } + + ranks := make([]float64, numVertices) + initialRank := 1.0 / float64(numVertices) + for i := range ranks { + ranks[i] = initialRank + } + + outDegree := make([]uint32, numVertices) + for _, edge := range edges { + src := edge[0] Review Comment: ‼️ The Go fallback counts an edge in `outDegree` when only `src` is valid, but the propagation loop later requires both endpoints to be valid. With `numVertices=2` and an edge `(0, 99)`, vertex 0 divides its rank by an edge that contributes nothing, so the fallback result loses mass and diverges from the Rust path. Please validate both endpoints before counting, or reject invalid edges with an error. ########## computer-rust/src/ffi/c_api.rs: ########## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You me obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::kernel::csr::CsrGraph; +use crate::kernel::pagerank::PageRankKernel; +use crate::kernel::sssp::SsspKernel; +use crate::RUST_KERNEL_VERSION; +use std::ffi::CString; +use std::os::raw::c_char; +use std::ptr; +use std::slice; + +pub struct GraphBuilder { + num_vertices: u32, + edges: Vec<(u32, u32, f64)>, + csr: Option<CsrGraph>, +} + +#[no_mangle] +pub extern "C" fn computer_graph_create(num_vertices: u32) -> *mut GraphBuilder { + let builder = Box::new(GraphBuilder { + num_vertices, + edges: Vec::new(), + csr: None, + }); + Box::into_raw(builder) +} + +#[no_mangle] +pub extern "C" fn computer_graph_add_edge( + handle: *mut GraphBuilder, + src: u32, + dst: u32, + weight: f64, +) -> i32 { + if handle.is_null() { + return -1; + } + let builder = unsafe { &mut *handle }; + builder.edges.push((src, dst, weight)); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_finalize(handle: *mut GraphBuilder) -> i32 { + if handle.is_null() { + return -1; + } + let builder = unsafe { &mut *handle }; + let csr = CsrGraph::from_edges(builder.num_vertices, &builder.edges); + builder.csr = Some(csr); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_compute_pagerank( + handle: *const GraphBuilder, + damping_factor: f64, + max_iterations: u32, + tolerance: f64, + out_scores: *mut f64, + out_capacity: u32, +) -> i32 { + if handle.is_null() || out_scores.is_null() { + return -1; + } + let builder = unsafe { &*handle }; + let csr = match &builder.csr { + Some(c) => c, + None => return -2, + }; + + if out_capacity < csr.num_vertices() { + return -3; + } + + let kernel = PageRankKernel::new(damping_factor, max_iterations, tolerance); + let ranks = kernel.compute(csr); + + let dest_slice = unsafe { slice::from_raw_parts_mut(out_scores, ranks.len()) }; + dest_slice.copy_from_slice(&ranks); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_compute_sssp( + handle: *const GraphBuilder, + source_vertex: u32, + out_distances: *mut f64, + out_capacity: u32, +) -> i32 { + if handle.is_null() || out_distances.is_null() { + return -1; + } + let builder = unsafe { &*handle }; + let csr = match &builder.csr { + Some(c) => c, + None => return -2, + }; + + if out_capacity < csr.num_vertices() { + return -3; + } + + let distances = SsspKernel::compute(csr, source_vertex); Review Comment: ⚠️ An out-of-range `source_vertex` is passed to `SsspKernel::compute()`, which returns an all-`INFINITY` vector, and the FFI function still returns `0`. This is indistinguishable from a valid graph whose vertices are all unreachable. Please validate the source at the C boundary and return a documented error code. ########## computer-rust/src/fixtures/tolerance.rs: ########## @@ -0,0 +1,77 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +pub struct DifferentialTolerance; + +impl DifferentialTolerance { + pub fn l1_distance(actual: &[f64], expected: &[f64]) -> Result<f64, String> { + if actual.len() != expected.len() { + return Err(format!( + "Vector length mismatch: actual len {}, expected len {}", + actual.len(), + expected.len() + )); + } + + let l1: f64 = actual + .iter() + .zip(expected.iter()) + .map(|(a, b)| (a - b).abs()) + .sum(); + + Ok(l1) + } + + pub fn assert_parity(actual: &[f64], expected: &[f64], epsilon: f64) -> Result<(), String> { + if actual.len() != expected.len() { + return Err(format!( + "Vector length mismatch: actual len {}, expected len {}", + actual.len(), + expected.len() + )); + } + + for i in 0..actual.len() { + let diff = (actual[i] - expected[i]).abs(); + if diff > epsilon { Review Comment: ⚠️ `NaN > epsilon` is false, so `assert_parity([f64::NAN], [0.0], epsilon)` returns `Ok(())`; `l1_distance()` likewise returns `Ok(NaN)`. A non-finite kernel result can therefore pass the differential fixture. Please reject non-finite inputs/differences and add NaN/Infinity regression cases. ########## computer-rust/src/ffi/c_api.rs: ########## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You me obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::kernel::csr::CsrGraph; +use crate::kernel::pagerank::PageRankKernel; +use crate::kernel::sssp::SsspKernel; +use crate::RUST_KERNEL_VERSION; +use std::ffi::CString; +use std::os::raw::c_char; +use std::ptr; +use std::slice; + +pub struct GraphBuilder { + num_vertices: u32, + edges: Vec<(u32, u32, f64)>, + csr: Option<CsrGraph>, +} + +#[no_mangle] +pub extern "C" fn computer_graph_create(num_vertices: u32) -> *mut GraphBuilder { + let builder = Box::new(GraphBuilder { + num_vertices, + edges: Vec::new(), + csr: None, + }); + Box::into_raw(builder) +} + +#[no_mangle] +pub extern "C" fn computer_graph_add_edge( + handle: *mut GraphBuilder, + src: u32, + dst: u32, + weight: f64, +) -> i32 { + if handle.is_null() { + return -1; + } + let builder = unsafe { &mut *handle }; + builder.edges.push((src, dst, weight)); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_finalize(handle: *mut GraphBuilder) -> i32 { + if handle.is_null() { + return -1; + } + let builder = unsafe { &mut *handle }; + let csr = CsrGraph::from_edges(builder.num_vertices, &builder.edges); + builder.csr = Some(csr); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_compute_pagerank( + handle: *const GraphBuilder, + damping_factor: f64, + max_iterations: u32, + tolerance: f64, + out_scores: *mut f64, + out_capacity: u32, +) -> i32 { + if handle.is_null() || out_scores.is_null() { + return -1; + } + let builder = unsafe { &*handle }; + let csr = match &builder.csr { + Some(c) => c, + None => return -2, + }; + + if out_capacity < csr.num_vertices() { + return -3; + } + + let kernel = PageRankKernel::new(damping_factor, max_iterations, tolerance); + let ranks = kernel.compute(csr); + + let dest_slice = unsafe { slice::from_raw_parts_mut(out_scores, ranks.len()) }; + dest_slice.copy_from_slice(&ranks); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_compute_sssp( + handle: *const GraphBuilder, + source_vertex: u32, + out_distances: *mut f64, + out_capacity: u32, +) -> i32 { + if handle.is_null() || out_distances.is_null() { + return -1; + } + let builder = unsafe { &*handle }; + let csr = match &builder.csr { + Some(c) => c, + None => return -2, + }; + + if out_capacity < csr.num_vertices() { + return -3; + } + + let distances = SsspKernel::compute(csr, source_vertex); + + let dest_slice = unsafe { slice::from_raw_parts_mut(out_distances, distances.len()) }; + dest_slice.copy_from_slice(&distances); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_free(handle: *mut GraphBuilder) { + if !handle.is_null() { + unsafe { + let _ = Box::from_raw(handle); + } + } +} + +#[no_mangle] +pub extern "C" fn computer_kernel_version() -> *const c_char { + thread_local! { Review Comment: ⚠️ `computer_kernel_version()` returns a pointer into a thread-local `CString`; that pointer becomes invalid when the calling thread exits, and the header does not document the borrowed lifetime or provide a copy/free contract. A C caller that stores the pointer or passes it across threads can use freed memory. Please return process-lifetime static storage or expose an explicit copy API and document ownership. ########## vermeer/apps/compute/rust_bridge_test.go: ########## @@ -0,0 +1,55 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one or more +contributor license agreements. See the NOTICE file distributed with this +work for additional information regarding copyright ownership. The ASF +licenses this file to You under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +License for the specific language governing permissions and limitations +under the License. +*/ + +package compute + +import ( + "math" + "testing" +) + +func TestRustBridgePageRank(t *testing.T) { Review Comment: ⚠️ The added Go bridge test is not exercised by the repository CI workflow, which builds Vermeer but does not run `go test`. Please add at least `go test ./apps/compute` (and a native-path job when bindings exist) so fallback behavior is continuously verified. ########## computer-rust/src/ffi/c_api.rs: ########## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You me obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::kernel::csr::CsrGraph; +use crate::kernel::pagerank::PageRankKernel; +use crate::kernel::sssp::SsspKernel; +use crate::RUST_KERNEL_VERSION; +use std::ffi::CString; +use std::os::raw::c_char; +use std::ptr; +use std::slice; + +pub struct GraphBuilder { + num_vertices: u32, + edges: Vec<(u32, u32, f64)>, + csr: Option<CsrGraph>, +} + +#[no_mangle] +pub extern "C" fn computer_graph_create(num_vertices: u32) -> *mut GraphBuilder { + let builder = Box::new(GraphBuilder { + num_vertices, + edges: Vec::new(), + csr: None, + }); + Box::into_raw(builder) +} + +#[no_mangle] +pub extern "C" fn computer_graph_add_edge( + handle: *mut GraphBuilder, + src: u32, + dst: u32, + weight: f64, +) -> i32 { + if handle.is_null() { + return -1; + } + let builder = unsafe { &mut *handle }; + builder.edges.push((src, dst, weight)); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_finalize(handle: *mut GraphBuilder) -> i32 { + if handle.is_null() { + return -1; + } + let builder = unsafe { &mut *handle }; + let csr = CsrGraph::from_edges(builder.num_vertices, &builder.edges); + builder.csr = Some(csr); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_compute_pagerank( + handle: *const GraphBuilder, + damping_factor: f64, + max_iterations: u32, + tolerance: f64, + out_scores: *mut f64, + out_capacity: u32, +) -> i32 { + if handle.is_null() || out_scores.is_null() { + return -1; + } + let builder = unsafe { &*handle }; + let csr = match &builder.csr { + Some(c) => c, + None => return -2, + }; + + if out_capacity < csr.num_vertices() { + return -3; + } + + let kernel = PageRankKernel::new(damping_factor, max_iterations, tolerance); + let ranks = kernel.compute(csr); + + let dest_slice = unsafe { slice::from_raw_parts_mut(out_scores, ranks.len()) }; + dest_slice.copy_from_slice(&ranks); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_compute_sssp( + handle: *const GraphBuilder, + source_vertex: u32, + out_distances: *mut f64, + out_capacity: u32, +) -> i32 { + if handle.is_null() || out_distances.is_null() { + return -1; + } + let builder = unsafe { &*handle }; + let csr = match &builder.csr { + Some(c) => c, + None => return -2, + }; + + if out_capacity < csr.num_vertices() { + return -3; + } + + let distances = SsspKernel::compute(csr, source_vertex); + + let dest_slice = unsafe { slice::from_raw_parts_mut(out_distances, distances.len()) }; + dest_slice.copy_from_slice(&distances); + 0 +} + +#[no_mangle] +pub extern "C" fn computer_graph_free(handle: *mut GraphBuilder) { + if !handle.is_null() { + unsafe { + let _ = Box::from_raw(handle); + } + } +} + +#[no_mangle] +pub extern "C" fn computer_kernel_version() -> *const c_char { + thread_local! { + static VERSION_C_STR: CString = CString::new(RUST_KERNEL_VERSION).unwrap(); + } + VERSION_C_STR.with(|c_str| c_str.as_ptr()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_c_api_flow() { + let handle = computer_graph_create(3); + assert!(!handle.is_null()); + + assert_eq!(computer_graph_add_edge(handle, 0, 1, 1.0), 0); + assert_eq!(computer_graph_add_edge(handle, 1, 2, 1.0), 0); + assert_eq!(computer_graph_add_edge(handle, 2, 0, 1.0), 0); + + assert_eq!(computer_graph_finalize(handle), 0); + + let mut scores = vec![0.0; 3]; + assert_eq!( Review Comment: ⚠️ The C-ABI flow test checks only that PageRank and SSSP return status `0`; it never checks the contents of `scores` or `dists`. A function that writes no output or computes incorrect distances would still pass. Please assert known PageRank values/sum and concrete SSSP distances, including an unreachable vertex. ########## computer-rust/src/ffi/c_api.rs: ########## @@ -0,0 +1,175 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You me obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::kernel::csr::CsrGraph; +use crate::kernel::pagerank::PageRankKernel; +use crate::kernel::sssp::SsspKernel; +use crate::RUST_KERNEL_VERSION; +use std::ffi::CString; +use std::os::raw::c_char; +use std::ptr; Review Comment: ‼️ `std::ptr` is unused in this file, while the new workflow runs `cargo clippy --all-targets -- -D warnings`. Once the workflow startup issue is fixed, this import will fail the quality gate. Please remove it and rerun Clippy. ########## computer-rust/src/kernel/pagerank.rs: ########## @@ -0,0 +1,103 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +use crate::kernel::csr::CsrGraph; + +#[derive(Debug, Clone)] +pub struct PageRankKernel { + damping_factor: f64, + max_iterations: u32, + tolerance: f64, +} + +impl PageRankKernel { + pub fn new(damping_factor: f64, max_iterations: u32, tolerance: f64) -> Self { + Self { + damping_factor, + max_iterations, + tolerance, + } + } + + pub fn compute(&self, graph: &CsrGraph) -> Vec<f64> { + let num_vertices = graph.num_vertices() as usize; + if num_vertices == 0 { + return Vec::new(); + } + + let initial_rank = 1.0 / (num_vertices as f64); + let mut ranks = vec![initial_rank; num_vertices]; + let mut next_ranks = vec![0.0; num_vertices]; + + let teleport = (1.0 - self.damping_factor) / (num_vertices as f64); + + for _iter in 0..self.max_iterations { + next_ranks.fill(0.0); + let mut dangling_sum = 0.0; + + for v in 0..num_vertices { + let out_degree = graph.out_degree(v as u32); + if out_degree == 0 { + dangling_sum += ranks[v]; + } else { + let share = ranks[v] / (out_degree as f64); + let (neighbors, _) = graph.out_edges(v as u32); + for &target in neighbors { + next_ranks[target as usize] += share; + } + } + } + + let dangling_share = self.damping_factor * (dangling_sum / (num_vertices as f64)); + let mut max_diff = 0.0f64; + + for v in 0..num_vertices { + let new_rank = teleport + dangling_share + self.damping_factor * next_ranks[v]; + let diff = (new_rank - ranks[v]).abs(); + if diff > max_diff { + max_diff = diff; + } + ranks[v] = new_rank; + } + + if max_diff < self.tolerance { + break; + } + } + + ranks + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_pagerank_computation() { + let edges = vec![(0, 1, 1.0), (1, 2, 1.0), (2, 0, 1.0)]; Review Comment: ⚠️ The PageRank test uses only a symmetric three-cycle, whose expected vector is uniform even if edge propagation is broken or the topology is ignored. Please add an asymmetric graph with a dangling vertex and assert a fixed reference result so transition and dangling-node handling are actually exercised. ########## computer/computer-test/src/main/java/org/apache/hugegraph/computer/core/rust/RustKernelBridgeTest.java: ########## @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hugegraph.computer.core.rust; + +import org.junit.Assert; +import org.junit.Test; + +public class RustKernelBridgeTest { + + @Test Review Comment: ⚠️ This new Java test is not included by the module's unit-test execution: `computer-test/pom.xml` includes only `**/UnitTestSuite.java`, and `UnitTestSuite` does not reference `RustKernelBridgeTest`. The class can compile while its fallback regression never runs in CI. Please add it to the suite or configure an explicit Surefire include, then verify the test count. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
