-
Notifications
You must be signed in to change notification settings - Fork 0
Fix python validation TODOs #263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
b9534f5
03661a4
faf56a8
c3118fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,3 +39,4 @@ validation/references/ | |
| # Example/optimiser output directories (all crates) | ||
| outputs/ | ||
| report/ | ||
| .venv/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,8 +7,8 @@ | |
| //! - Historical performance trend analysis | ||
| //! - Performance validation against requirements | ||
|
|
||
| use crate::BenchmarkConfig; | ||
| use criterion::{black_box, Criterion}; | ||
| use cfd_validation::benchmarking::BenchmarkConfig; | ||
| use criterion::{black_box, criterion_group, criterion_main, Criterion}; | ||
| use std::collections::HashMap; | ||
| use std::fs; | ||
| use std::path::{Path, PathBuf}; | ||
|
|
@@ -514,7 +514,9 @@ fn run_memory_allocation_benchmark(size: usize) -> (Duration, Duration) { | |
|
|
||
| for _ in 0..10 { | ||
| let start = std::time::Instant::now(); | ||
| let data = black_box(vec![0.0f64; size * size]); | ||
| // Prevent OOM by capping size for the test (size=1000 => 1M elements ~ 8MB) | ||
| let capped_size = size.min(1000); | ||
| let data = black_box(vec![0.0f64; capped_size * capped_size]); | ||
| let _sum = data.iter().sum::<f64>(); | ||
| drop(data); | ||
| times.push(start.elapsed()); | ||
|
|
@@ -542,9 +544,10 @@ fn run_cfd_computation_benchmark(size: usize) -> (Duration, Duration) { | |
| let start = std::time::Instant::now(); | ||
| use nalgebra::DMatrix; | ||
|
|
||
| let mut field = DMatrix::<f64>::zeros(size, size); | ||
| for i in 0..size.min(20) { | ||
| for j in 0..size.min(20) { | ||
| let capped_size = size.min(1000); | ||
| let mut field = DMatrix::<f64>::zeros(capped_size, capped_size); | ||
| for i in 0..capped_size.min(20) { | ||
| for j in 0..capped_size.min(20) { | ||
| field[(i, j)] = (i as f64 * j as f64).sin(); | ||
| } | ||
| } | ||
|
Comment on lines
+547
to
553
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same throughput mismatch issue applies here. The CFD computation benchmark has the same problem: 🤖 Prompt for AI Agents |
||
|
|
@@ -647,3 +650,11 @@ pub fn detect_performance_regressions(metrics: &[crate::PerformanceMetrics]) { | |
| // detailed statistical analysis and automated alerting. | ||
| println!("Analyzing performance for {} operations...", metrics.len()); | ||
| } | ||
|
|
||
| pub fn regression_detection_benchmark(c: &mut Criterion) { | ||
| let config = BenchmarkConfig::default(); | ||
| benchmark_regression_detection(c, &config); | ||
| } | ||
|
|
||
| criterion_group!(benches, regression_detection_benchmark); | ||
| criterion_main!(benches); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| //! Cavitation model wrappers for `PyO3` | ||
|
|
||
| use cfd_core::physics::cavitation::RayleighPlesset; | ||
| use pyo3::prelude::*; | ||
|
|
||
| /// Calculate critical Blake radius for unstable growth | ||
| #[pyfunction] | ||
| #[pyo3(name = "blake_critical_radius")] | ||
| pub fn py_blake_critical_radius(p_inf: f64, p_v: f64, sigma: f64) -> f64 { | ||
| // R_c = 0.85 * (2σ/(p_∞ - p_v)) | ||
| let model = RayleighPlesset { | ||
| initial_radius: 10e-6, // Dummy value | ||
| liquid_density: 997.0, // Dummy value | ||
| liquid_viscosity: 0.001, // Dummy value | ||
| surface_tension: sigma, | ||
| vapor_pressure: p_v, | ||
| polytropic_index: 1.4, // Dummy value | ||
| }; | ||
| model.blake_critical_radius(p_inf) | ||
| } | ||
|
|
||
| /// Calculate Blake threshold pressure | ||
| #[pyfunction] | ||
| #[pyo3(name = "blake_threshold")] | ||
| pub fn py_blake_threshold(p_inf: f64, p_v: f64, sigma: f64) -> f64 { | ||
| let r_critical = py_blake_critical_radius(p_inf, p_v, sigma); | ||
| p_v + (4.0 / 3.0) * sigma / r_critical | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| //! Hemolysis model wrappers for `PyO3` | ||
|
|
||
| use cfd_core::physics::hemolysis::HemolysisModel as RustHemolysisModel; | ||
| use pyo3::prelude::*; | ||
|
|
||
| #[pyclass(name = "HemolysisModel")] | ||
| pub struct PyHemolysisModel { | ||
| inner: RustHemolysisModel, | ||
| } | ||
|
|
||
| #[pymethods] | ||
| impl PyHemolysisModel { | ||
| /// Create Giersiepen model with standard constants | ||
| #[staticmethod] | ||
| pub fn giersiepen_standard() -> Self { | ||
| PyHemolysisModel { | ||
| inner: RustHemolysisModel::giersiepen_standard(), | ||
| } | ||
| } | ||
|
|
||
| /// Create Giersiepen model for turbulent flow | ||
| #[staticmethod] | ||
| pub fn giersiepen_turbulent() -> Self { | ||
| PyHemolysisModel { | ||
| inner: RustHemolysisModel::giersiepen_turbulent(), | ||
| } | ||
| } | ||
|
|
||
| /// Create Giersiepen model for laminar flow | ||
| #[staticmethod] | ||
| pub fn giersiepen_laminar() -> Self { | ||
| PyHemolysisModel { | ||
| inner: RustHemolysisModel::giersiepen_laminar(), | ||
| } | ||
| } | ||
|
|
||
| /// Create Zhang model for Couette flow | ||
| #[staticmethod] | ||
| pub fn zhang() -> Self { | ||
| PyHemolysisModel { | ||
| inner: RustHemolysisModel::zhang(), | ||
| } | ||
| } | ||
|
|
||
| /// Create Heuser-Opitz threshold model | ||
| #[staticmethod] | ||
| pub fn heuser_opitz() -> Self { | ||
| PyHemolysisModel { | ||
| inner: RustHemolysisModel::heuser_opitz(), | ||
| } | ||
| } | ||
|
|
||
| /// Calculate blood damage index from shear stress and exposure time | ||
| pub fn damage_index(&self, shear_stress: f64, exposure_time: f64) -> PyResult<f64> { | ||
| self.inner.damage_index(shear_stress, exposure_time).map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string())) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| { | ||
| "timestamp": 1773021517, | ||
| "benchmarks": { | ||
| "memory_allocation_100000": { | ||
| "name": "memory_allocation_100000", | ||
| "mean_time_ns": 401641, | ||
| "std_dev_ns": 26945, | ||
| "throughput": 24897856543530.168, | ||
| "samples": 100, | ||
| "confidence_interval": [ | ||
| 396359, | ||
| 406922 | ||
| ] | ||
| }, | ||
| "cfd_computation_1000": { | ||
| "name": "cfd_computation_1000", | ||
| "mean_time_ns": 433924, | ||
| "std_dev_ns": 91054, | ||
| "throughput": 2304551027.3688483, | ||
| "samples": 100, | ||
| "confidence_interval": [ | ||
| 416077, | ||
| 451770 | ||
| ] | ||
| }, | ||
| "memory_allocation_10000": { | ||
| "name": "memory_allocation_10000", | ||
| "mean_time_ns": 473367, | ||
| "std_dev_ns": 134256, | ||
| "throughput": 211252579922.1323, | ||
| "samples": 100, | ||
| "confidence_interval": [ | ||
| 447052, | ||
| 499681 | ||
| ] | ||
| }, | ||
| "cfd_computation_10000": { | ||
| "name": "cfd_computation_10000", | ||
| "mean_time_ns": 434459, | ||
| "std_dev_ns": 43454, | ||
| "throughput": 230171316510.87906, | ||
| "samples": 100, | ||
| "confidence_interval": [ | ||
| 425942, | ||
| 442975 | ||
| ] | ||
| }, | ||
| "memory_allocation_1000": { | ||
| "name": "memory_allocation_1000", | ||
| "mean_time_ns": 1069666, | ||
| "std_dev_ns": 1819851, | ||
| "throughput": 934871258.8789399, | ||
| "samples": 100, | ||
| "confidence_interval": [ | ||
| 712975, | ||
| 1426356 | ||
| ] | ||
| }, | ||
| "cfd_computation_100000": { | ||
| "name": "cfd_computation_100000", | ||
| "mean_time_ns": 442298, | ||
| "std_dev_ns": 101958, | ||
| "throughput": 22609191088361.242, | ||
| "samples": 100, | ||
| "confidence_interval": [ | ||
| 422314, | ||
| 462281 | ||
| ] | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throughput metric becomes misleading after capping.
The capping prevents OOM, which is good. However, the benchmark is still named
memory_allocation_{size}and throughput is calculated using the original uncappedsize(seecalculate_throughput(result.0, size)at line 491), while actual work usescapped_size.For
size=100000: throughput is computed as(100000² / time)but only(1000² = 1M)elements are processed. This produces inflated throughput values (~10,000x higher than reality) in the baseline files.Consider either:
capped_sizeto the throughput calculation and update benchmark names to reflect actual work, orOption 1: Use capped_size for accurate metrics
fn run_memory_allocation_benchmark(size: usize) -> (Duration, Duration) { let mut times = Vec::new(); + let capped_size = size.min(1000); for _ in 0..10 { let start = std::time::Instant::now(); - // Prevent OOM by capping size for the test (size=1000 => 1M elements ~ 8MB) - let capped_size = size.min(1000); let data = black_box(vec![0.0f64; capped_size * capped_size]); let _sum = data.iter().sum::<f64>(); drop(data); times.push(start.elapsed()); } // ... rest unchanged ... + (mean, std_dev, capped_size) // Return capped_size for throughput calc }Then update callers to use the returned capped size for throughput calculation.
🤖 Prompt for AI Agents