Skip to content

Commit 701032a

Browse files
authored
Merge pull request #180 from acgetchell/refactor/165-bench-or-abort
refactor(bench): consolidate setup abort handling
2 parents 2ffa19c + b9e453e commit 701032a

12 files changed

Lines changed: 260 additions & 243 deletions

File tree

.coderabbit.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
2+
---
3+
reviews:
4+
request_changes_workflow: true

.github/workflows/rust-clippy.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,12 @@ jobs:
5656
- name: Run clippy with SARIF output
5757
run: |
5858
set -euo pipefail
59+
# Lint levels are owned by Cargo.toml.
5960
cargo clippy \
6061
--workspace \
6162
--all-targets \
6263
--all-features \
63-
--message-format=json \
64-
-- -W clippy::pedantic -W clippy::nursery -W clippy::cargo | \
64+
--message-format=json | \
6565
clippy-sarif | \
6666
tee rust-clippy-results.sarif | \
6767
sarif-fmt

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,10 +133,10 @@ invariant over the convenient edit.
133133
### Testing mirrors the principles
134134

135135
- Unit tests cover known values, error paths, and dimension-generic
136+
correctness across D=2..=5 (see **Dimension Coverage** below).
136137
- Error-path tests match the exact variant, typed reason/origin/location, and
137138
structured fields; do not replace an unexpected error with a numeric sentinel
138139
or assert only `is_err()`.
139-
correctness across D=2..=5 (see **Dimension Coverage** below).
140140
- Proptests under `tests/proptest_*.rs` cover algebraic invariants
141141
(round-trip, residual, sign agreement) — not just "does it not panic".
142142
- Adversarial inputs (near-singular, large-entry, Hilbert-style

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,5 +96,8 @@ bare_urls = "deny"
9696
broken_intra_doc_links = "deny"
9797

9898
[lints.clippy]
99+
cargo = { level = "warn", priority = -1 }
99100
extra_unused_type_parameters = "warn"
101+
nursery = { level = "warn", priority = -1 }
100102
pedantic = { level = "warn", priority = -1 }
103+
redundant_pub_crate = { level = "allow", priority = 0 }

benches/common/bench_utils.rs

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
#![forbid(unsafe_code)]
2+
3+
//! Shared helpers for benchmark operations that cannot recover from failure.
4+
5+
use std::fmt::Display;
6+
7+
/// Convert a fallible benchmark operation into its successful value or abort.
8+
pub(crate) trait OrAbort {
9+
/// Successful value produced by the operation.
10+
type Output;
11+
12+
/// Return the successful value or panic with the named operation.
13+
///
14+
/// # Panics
15+
///
16+
/// Panics when the benchmark operation contains an error or no value.
17+
fn or_abort(self, operation: &str) -> Self::Output;
18+
}
19+
20+
impl<T, E: Display> OrAbort for Result<T, E> {
21+
type Output = T;
22+
23+
fn or_abort(self, operation: &str) -> Self::Output {
24+
match self {
25+
Ok(value) => value,
26+
Err(err) => panic!("{operation} failed: {err}"),
27+
}
28+
}
29+
}
30+
31+
impl<T> OrAbort for Option<T> {
32+
type Output = T;
33+
34+
fn or_abort(self, operation: &str) -> Self::Output {
35+
self.unwrap_or_else(|| panic!("{operation} returned no result"))
36+
}
37+
}
38+
39+
#[cfg(test)]
40+
mod tests {
41+
#[test]
42+
fn returns_result_and_option_values() {
43+
assert_eq!(
44+
<Result<_, &str> as super::OrAbort>::or_abort(Ok(7), "successful result"),
45+
7
46+
);
47+
assert_eq!(
48+
<Option<_> as super::OrAbort>::or_abort(Some(11), "present option"),
49+
11
50+
);
51+
}
52+
53+
#[test]
54+
#[should_panic(expected = "fallible setup failed: fixture error")]
55+
fn result_error_preserves_context_and_error() {
56+
<Result<(), &str> as super::OrAbort>::or_abort(Err("fixture error"), "fallible setup");
57+
}
58+
59+
#[test]
60+
#[should_panic(expected = "optional setup returned no result")]
61+
fn missing_option_preserves_context() {
62+
<Option<()> as super::OrAbort>::or_abort(None, "optional setup");
63+
}
64+
}

benches/common/exact.rs

Lines changed: 50 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ use num_bigint::BigInt;
1212
use num_rational::BigRational;
1313
use num_traits::{FromPrimitive, Signed, ToPrimitive, Zero};
1414

15+
use crate::bench_utils::OrAbort;
16+
1517
/// Number of matrices in each deterministic random benchmark corpus.
1618
pub const RANDOM_INPUT_ARRAY_LEN: usize = 50;
1719
/// Stable global seed used to derive one random corpus per dimension.
@@ -147,14 +149,6 @@ impl<const D: usize> ValidatedExactInput<D> {
147149
}
148150
}
149151

150-
/// Return a successful fixture-construction result or panic with context.
151-
fn require_ok<T, E: Display>(result: Result<T, E>, operation: &str) -> T {
152-
match result {
153-
Ok(value) => value,
154-
Err(err) => panic!("{operation} failed: {err}"),
155-
}
156-
}
157-
158152
/// Return one matrix entry through the bounds-checked API shared by current and
159153
/// v0.4.3 releases.
160154
fn stored_matrix_entry<const D: usize>(matrix: &Matrix<D>, row: usize, col: usize) -> f64 {
@@ -172,10 +166,9 @@ fn checked_det_sign<const D: usize>(matrix: &Matrix<D>) -> i8 {
172166

173167
#[cfg(la_stack_v0_4_3_api)]
174168
fn checked_det_sign<const D: usize>(matrix: &Matrix<D>) -> i8 {
175-
require_ok(
176-
matrix.det_sign_exact(),
177-
"exact determinant sign oracle check",
178-
)
169+
matrix
170+
.det_sign_exact()
171+
.or_abort("exact determinant sign oracle check")
179172
}
180173

181174
/// Return a deterministic, strictly diagonally-dominant matrix entry.
@@ -223,19 +216,19 @@ pub fn make_vector_array<const D: usize>() -> [f64; D] {
223216
/// Derive a stable per-dimension seed from the global random benchmark seed.
224217
fn random_seed_for_dim<const D: usize>() -> u64 {
225218
let mut seed =
226-
0xC0DE_CAFE_D15C_A11Au64 ^ require_ok(u64::try_from(D), "dimension seed conversion");
219+
0xC0DE_CAFE_D15C_A11Au64 ^ u64::try_from(D).or_abort("dimension seed conversion");
227220
for (i, byte) in RANDOM_SEED.iter().copied().enumerate() {
228-
let shift = require_ok(u32::try_from((i % 8) * 8), "seed shift conversion");
221+
let shift = u32::try_from((i % 8) * 8).or_abort("seed shift conversion");
229222
seed ^= u64::from(byte) << shift;
230-
seed = seed.rotate_left(7) ^ require_ok(u64::try_from(i), "seed index conversion");
223+
seed = seed.rotate_left(7) ^ u64::try_from(i).or_abort("seed index conversion");
231224
}
232225
seed
233226
}
234227

235228
/// Build a fixed random corpus of finite, strictly diagonally-dominant inputs.
236229
pub fn make_random_input_corpus<const D: usize>() -> [ExactInput<D>; RANDOM_INPUT_ARRAY_LEN] {
237230
let mut rng = SplitMix64::new(random_seed_for_dim::<D>());
238-
let entry_range = require_ok(I16Range::try_new(-10, 10), "random integer range");
231+
let entry_range = I16Range::try_new(-10, 10).or_abort("random integer range");
239232
array::from_fn(|_| {
240233
let mut rows = [[0.0; D]; D];
241234
let mut diag = [0_i16; D];
@@ -251,7 +244,7 @@ pub fn make_random_input_corpus<const D: usize>() -> [ExactInput<D>; RANDOM_INPU
251244
}
252245

253246
let shift =
254-
f64::from(require_ok(u8::try_from(D), "dimension shift conversion")).mul_add(10.0, 1.0);
247+
f64::from(u8::try_from(D).or_abort("dimension shift conversion")).mul_add(10.0, 1.0);
255248
for (i, row) in rows.iter_mut().enumerate() {
256249
row[i] = if diag[i] >= 0 {
257250
f64::from(diag[i]) + shift
@@ -263,11 +256,8 @@ pub fn make_random_input_corpus<const D: usize>() -> [ExactInput<D>; RANDOM_INPU
263256
let rhs = from_fn(|_| f64::from(rng.next_i16(entry_range)));
264257

265258
ExactInput {
266-
matrix: require_ok(
267-
Matrix::<D>::try_from_rows(rows),
268-
"random matrix construction",
269-
),
270-
rhs: require_ok(Vector::<D>::try_new(rhs), "random RHS vector construction"),
259+
matrix: Matrix::<D>::try_from_rows(rows).or_abort("random matrix construction"),
260+
rhs: Vector::<D>::try_new(rhs).or_abort("random RHS vector construction"),
271261
}
272262
})
273263
}
@@ -276,33 +266,24 @@ pub fn make_random_input_corpus<const D: usize>() -> [ExactInput<D>; RANDOM_INPU
276266
pub fn near_singular_3x3_input() -> ExactInput<3> {
277267
let perturbation = f64::from_bits(0x3CD0_0000_0000_0000); // 2^-50
278268
ExactInput {
279-
matrix: require_ok(
280-
Matrix::<3>::try_from_rows([
281-
[1.0 + perturbation, 2.0, 3.0],
282-
[4.0, 5.0, 6.0],
283-
[7.0, 8.0, 9.0],
284-
]),
285-
"near-singular matrix construction",
286-
),
287-
rhs: require_ok(
288-
Vector::<3>::try_new([1.0, 2.0, 3.0]),
289-
"near-singular RHS vector construction",
290-
),
269+
matrix: Matrix::<3>::try_from_rows([
270+
[1.0 + perturbation, 2.0, 3.0],
271+
[4.0, 5.0, 6.0],
272+
[7.0, 8.0, 9.0],
273+
])
274+
.or_abort("near-singular matrix construction"),
275+
rhs: Vector::<3>::try_new([1.0, 2.0, 3.0])
276+
.or_abort("near-singular RHS vector construction"),
291277
}
292278
}
293279

294280
/// Build the fixed extreme-magnitude 3×3 benchmark input.
295281
pub fn large_entries_3x3_input() -> ExactInput<3> {
296282
let big = f64::MAX / 2.0;
297283
ExactInput {
298-
matrix: require_ok(
299-
Matrix::<3>::try_from_rows([[big, 1.0, 1.0], [1.0, big, 1.0], [1.0, 1.0, big]]),
300-
"large-entry matrix construction",
301-
),
302-
rhs: require_ok(
303-
Vector::<3>::try_new([1.0, 1.0, 1.0]),
304-
"large-entry RHS vector construction",
305-
),
284+
matrix: Matrix::<3>::try_from_rows([[big, 1.0, 1.0], [1.0, big, 1.0], [1.0, 1.0, big]])
285+
.or_abort("large-entry matrix construction"),
286+
rhs: Vector::<3>::try_new([1.0, 1.0, 1.0]).or_abort("large-entry RHS vector construction"),
306287
}
307288
}
308289

@@ -314,14 +295,8 @@ pub fn large_entries_3x3_input() -> ExactInput<3> {
314295
pub fn hilbert_input<const D: usize>() -> ExactInput<D> {
315296
let rows = from_fn(|r| from_fn(|c| 1.0 / ((r + c + 1) as f64)));
316297
ExactInput {
317-
matrix: require_ok(
318-
Matrix::<D>::try_from_rows(rows),
319-
"Hilbert matrix construction",
320-
),
321-
rhs: require_ok(
322-
Vector::<D>::try_new([1.0; D]),
323-
"Hilbert RHS vector construction",
324-
),
298+
matrix: Matrix::<D>::try_from_rows(rows).or_abort("Hilbert matrix construction"),
299+
rhs: Vector::<D>::try_new([1.0; D]).or_abort("Hilbert RHS vector construction"),
325300
}
326301
}
327302

@@ -528,13 +503,16 @@ fn assert_approximate_determinant(actual: f64, exact: &BigRational, operation: &
528503
/// dimension, or disagrees with the independent exact oracle.
529504
pub fn validate_f64_determinant_benchmarks<const D: usize>(input: &ValidatedExactInput<D>) {
530505
let exact = determinant_leibniz(input.matrix());
531-
let determinant = require_ok(input.matrix().det(), "f64 determinant oracle check");
506+
let determinant = input
507+
.matrix()
508+
.det()
509+
.or_abort("f64 determinant oracle check");
532510
assert_approximate_determinant(determinant, &exact, "f64 determinant");
533511

534-
let direct = require_ok(
535-
input.matrix().det_direct(),
536-
"direct f64 determinant oracle check",
537-
);
512+
let direct = input
513+
.matrix()
514+
.det_direct()
515+
.or_abort("direct f64 determinant oracle check");
538516
if D <= 4 {
539517
let Some(direct) = direct else {
540518
panic!("det_direct must support benchmark dimension {D}");
@@ -543,10 +521,10 @@ pub fn validate_f64_determinant_benchmarks<const D: usize>(input: &ValidatedExac
543521

544522
#[cfg(not(la_stack_v0_4_3_api))]
545523
{
546-
let estimate = require_ok(
547-
input.matrix().det_direct_with_errbound(),
548-
"combined direct determinant oracle check",
549-
);
524+
let estimate = input
525+
.matrix()
526+
.det_direct_with_errbound()
527+
.or_abort("combined direct determinant oracle check");
550528
let Some(estimate) = estimate else {
551529
panic!("the baseline fixture must have a certified D={D} determinant bound");
552530
};
@@ -563,11 +541,11 @@ pub fn validate_f64_determinant_benchmarks<const D: usize>(input: &ValidatedExac
563541

564542
#[cfg(not(la_stack_v0_4_3_api))]
565543
assert!(
566-
require_ok(
567-
input.matrix().det_direct_with_errbound(),
568-
"combined direct determinant scope check",
569-
)
570-
.is_none(),
544+
input
545+
.matrix()
546+
.det_direct_with_errbound()
547+
.or_abort("combined direct determinant scope check")
548+
.is_none(),
571549
"combined direct determinant unexpectedly supports D={D}",
572550
);
573551
}
@@ -599,7 +577,10 @@ fn assert_exact_residual<const D: usize>(input: &ExactInput<D>, solution: &[BigR
599577
pub fn validate_exact_fixture<const D: usize>(input: ExactInput<D>) -> ValidatedExactInput<D> {
600578
let determinant = determinant_leibniz(&input.matrix);
601579
assert_eq!(
602-
require_ok(input.matrix.det_exact(), "exact determinant oracle check"),
580+
input
581+
.matrix
582+
.det_exact()
583+
.or_abort("exact determinant oracle check"),
603584
determinant
604585
);
605586
assert_eq!(
@@ -609,10 +590,10 @@ pub fn validate_exact_fixture<const D: usize>(input: ExactInput<D>) -> Validated
609590
assert_strict_scalar(input.matrix.det_exact_f64(), &determinant, None);
610591
assert_rounded_scalar(input.matrix.det_exact_rounded_f64(), &determinant);
611592

612-
let solution = require_ok(
613-
input.matrix.solve_exact(input.rhs),
614-
"exact solve oracle check",
615-
);
593+
let solution = input
594+
.matrix
595+
.solve_exact(input.rhs)
596+
.or_abort("exact solve oracle check");
616597
assert_exact_residual(&input, &solution);
617598

618599
let strict_solution = input.matrix.solve_exact_f64(input.rhs);

0 commit comments

Comments
 (0)