imbajin commented on code in PR #359: URL: https://github.com/apache/hugegraph-computer/pull/359#discussion_r3763206217
########## .github/workflows/rust-ci.yml: ########## @@ -0,0 +1,75 @@ +# +# 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. +# +name: "Rust CI" + +on: + push: + branches: + - master + - /^release-.*$/ Review Comment: ‼️ GitHub Actions branch filters use glob syntax, but `/^release-.*$/` is rejected as an invalid branch name/pattern (`actionlint` reports the leading `/`, `^`, and trailing `/` as invalid); the exact-head Rust CI run 31351599715 ended in `startup_failure`, so formatting, clippy, tests, and release build never ran. Please use a valid glob such as `release-*` and rerun the workflow. ########## 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 Review Comment: ⚠️ The Apache header contains `You me obtain a copy`, which makes the exact-head `check-license-header` job fail on this file. Please correct the standard license text to `You may obtain a copy` and rerun the license check. ########## 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, Review Comment: ⚠️ The new Rust library is not reachable from the advertised Vermeer path: `NewRustKernelBridge` hard-codes `available: false`, and `ComputePageRank` always executes the Go fallback. The Java bridge likewise computes in Java and only declares `nativeGetVersion`, which does not match Rust's `computer_kernel_version` export. Please implement and test the JNI/CGO bindings and native-path selection, or document this PR as fallback-only instead of presenting an active Rust integration. ########## computer-rust/src/kernel/csr.rs: ########## @@ -0,0 +1,123 @@ +/* + * 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. + */ + +#[derive(Debug, Clone, Default)] +pub struct Edge { + pub target: u32, + pub weight: f64, +} + +#[derive(Debug, Clone)] +pub struct CsrGraph { + num_vertices: u32, + row_offsets: Vec<usize>, + column_indices: Vec<u32>, + edge_weights: Vec<f64>, +} + +impl CsrGraph { + pub fn new(num_vertices: u32) -> Self { + Self { + num_vertices, + row_offsets: vec![0; (num_vertices + 1) as usize], + column_indices: Vec::new(), + edge_weights: Vec::new(), + } + } + + pub fn from_edges(num_vertices: u32, edges: &[(u32, u32, f64)]) -> Self { + let mut degree = vec![0; num_vertices as usize]; + for &(src, _dst, _weight) in edges { + if src < num_vertices { Review Comment: ‼️ `degree` counts every edge whose source is in range, but the fill loop skips an out-of-range destination. For example, `from_edges(2, &[(0, 99, 1.0)])` allocates one slot and leaves it as the default `0 -> 0` edge, so PageRank/SSSP consume a topology that was never supplied. Please validate both endpoints when counting and filling, and return an error from the C API for invalid vertices. ########## computer-rust/src/kernel/sssp.rs: ########## @@ -0,0 +1,99 @@ +/* + * 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; +use std::cmp::Ordering; +use std::collections::BinaryHeap; + +#[derive(Copy, Clone, PartialEq)] +struct State { + cost: f64, + position: u32, +} + +impl Eq for State {} + +impl Ord for State { + fn cmp(&self, other: &Self) -> Ordering { + other.cost.partial_cmp(&self.cost).unwrap_or(Ordering::Equal) + } +} + +impl PartialOrd for State { + fn partial_cmp(&self, other: &Self) -> Option<Ordering> { + Some(self.cmp(other)) + } +} + +pub struct SsspKernel; + +impl SsspKernel { + pub fn compute(graph: &CsrGraph, source: u32) -> Vec<f64> { + let num_vertices = graph.num_vertices() as usize; + let mut dist = vec![f64::INFINITY; num_vertices]; + let mut heap = BinaryHeap::new(); + + if (source as usize) >= num_vertices { + return dist; + } + + dist[source as usize] = 0.0; + heap.push(State { + cost: 0.0, + position: source, + }); + + while let Some(State { cost, position }) = heap.pop() { + if cost > dist[position as usize] { + continue; + } + + let (neighbors, weights) = graph.out_edges(position); + for i in 0..neighbors.len() { + let next_target = neighbors[i]; + let next_cost = cost + weights[i]; Review Comment: ‼️ This Dijkstra loop accepts negative weights and has no negative-cycle detection. A graph containing `0 -> 1 = -1` and `1 -> 0 = -1` keeps lowering both distances and pushing new heap entries, so the exported SSSP call can run without termination and exhaust CPU/memory. Please reject negative/non-finite weights at the API boundary or use an algorithm that detects negative cycles. -- 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]
