Skip to content

Commit 717d5cf

Browse files
acgetchelloz-agent
andcommitted
fix!: return Result from det_sign_exact, rename NonFinite field
BREAKING CHANGE: two public API changes to LaError and det_sign_exact. - `LaError::NonFinite { pivot_col }` renamed to `LaError::NonFinite { col }` to reflect that NonFinite errors are not always pivot-related (e.g. det_sign_exact, det). - `Matrix::det_sign_exact()` now returns `Result<i8, LaError>` instead of `i8`. Returns `Err(LaError::NonFinite { col })` for NaN/infinite entries instead of panicking. This aligns with lu(), ldlt(), and det(). - When det_direct() overflows to infinity but all entries are finite, the fast filter is now skipped gracefully and Bareiss computes the correct exact sign (previously panicked). Co-Authored-By: Oz <oz-agent@warp.dev>
1 parent 214d5ae commit 717d5cf

8 files changed

Lines changed: 103 additions & 90 deletions

File tree

AGENTS.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,8 @@ When making changes in this repo, prioritize (in order):
6969
## Publishing note
7070

7171
- If you publish this crate to crates.io, prefer updating documentation *before* publishing a new version (doc-only changes still require a version bump on crates.io).
72+
73+
## Editing tools policy
74+
75+
- Never use `sed`, `awk`, `python`, or `perl` to edit code or write file changes.
76+
- These tools may be used for read-only inspection, parsing, or analysis, but never for writing.

examples/exact_sign_3x3.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ fn main() {
2323
[7.0, 8.0, 9.0],
2424
]);
2525

26-
let sign = m.det_sign_exact();
26+
let sign = m.det_sign_exact().unwrap();
2727
let det_f64 = m.det(DEFAULT_PIVOT_TOL).unwrap();
2828

2929
println!("Near-singular 3×3 matrix (perturbation = 2^-50 ≈ {perturbation:.2e}):");

src/exact.rs

Lines changed: 68 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
use num_bigint::{BigInt, Sign};
1818
use num_rational::BigRational;
1919

20+
use crate::LaError;
2021
use crate::matrix::Matrix;
2122

2223
// ---------------------------------------------------------------------------
@@ -195,36 +196,47 @@ impl<const D: usize> Matrix<D> {
195196
/// [7.0, 8.0, 9.0],
196197
/// ]);
197198
/// // This matrix is singular (row 3 = row 1 + row 2 in exact arithmetic).
198-
/// assert_eq!(m.det_sign_exact(), 0);
199+
/// assert_eq!(m.det_sign_exact().unwrap(), 0);
199200
///
200-
/// assert_eq!(Matrix::<3>::identity().det_sign_exact(), 1);
201+
/// assert_eq!(Matrix::<3>::identity().det_sign_exact().unwrap(), 1);
201202
/// ```
202203
///
203-
/// # Panics
204-
/// Panics if any matrix entry is NaN or infinite.
205-
#[must_use]
206-
pub fn det_sign_exact(&self) -> i8 {
204+
/// # Errors
205+
/// Returns [`LaError::NonFinite`] if any matrix entry is NaN or infinite.
206+
#[inline]
207+
pub fn det_sign_exact(&self) -> Result<i8, LaError> {
208+
// Validate all entries are finite before any arithmetic.
209+
for r in 0..D {
210+
for c in 0..D {
211+
if !self.rows[r][c].is_finite() {
212+
return Err(LaError::NonFinite { col: c });
213+
}
214+
}
215+
}
216+
207217
// Stage 1: f64 fast filter for D ≤ 4.
208218
if let (Some(det_f64), Some(err)) = (self.det_direct(), det_errbound(self)) {
209-
assert!(
210-
det_f64.is_finite(),
211-
"non-finite matrix entry in det_sign_exact"
212-
);
213-
if det_f64 > err {
214-
return 1;
215-
}
216-
if det_f64 < -err {
217-
return -1;
219+
// When entries are large (e.g. near f64::MAX) the determinant can
220+
// overflow to infinity even though every individual entry is finite.
221+
// In that case the fast filter is inconclusive; fall through to the
222+
// exact Bareiss path.
223+
if det_f64.is_finite() {
224+
if det_f64 > err {
225+
return Ok(1);
226+
}
227+
if det_f64 < -err {
228+
return Ok(-1);
229+
}
218230
}
219231
}
220232

221233
// Stage 2: exact Bareiss fallback.
222234
let det = bareiss_det(self);
223-
match det.numer().sign() {
235+
Ok(match det.numer().sign() {
224236
Sign::Plus => 1,
225237
Sign::Minus => -1,
226238
Sign::NoSign => 0,
227-
}
239+
})
228240
}
229241
}
230242

@@ -235,45 +247,45 @@ mod tests {
235247

236248
#[test]
237249
fn det_sign_exact_d0_is_positive() {
238-
assert_eq!(Matrix::<0>::zero().det_sign_exact(), 1);
250+
assert_eq!(Matrix::<0>::zero().det_sign_exact().unwrap(), 1);
239251
}
240252

241253
#[test]
242254
fn det_sign_exact_d1_positive() {
243255
let m = Matrix::<1>::from_rows([[42.0]]);
244-
assert_eq!(m.det_sign_exact(), 1);
256+
assert_eq!(m.det_sign_exact().unwrap(), 1);
245257
}
246258

247259
#[test]
248260
fn det_sign_exact_d1_negative() {
249261
let m = Matrix::<1>::from_rows([[-3.5]]);
250-
assert_eq!(m.det_sign_exact(), -1);
262+
assert_eq!(m.det_sign_exact().unwrap(), -1);
251263
}
252264

253265
#[test]
254266
fn det_sign_exact_d1_zero() {
255267
let m = Matrix::<1>::from_rows([[0.0]]);
256-
assert_eq!(m.det_sign_exact(), 0);
268+
assert_eq!(m.det_sign_exact().unwrap(), 0);
257269
}
258270

259271
#[test]
260272
fn det_sign_exact_identity_2d() {
261-
assert_eq!(Matrix::<2>::identity().det_sign_exact(), 1);
273+
assert_eq!(Matrix::<2>::identity().det_sign_exact().unwrap(), 1);
262274
}
263275

264276
#[test]
265277
fn det_sign_exact_identity_3d() {
266-
assert_eq!(Matrix::<3>::identity().det_sign_exact(), 1);
278+
assert_eq!(Matrix::<3>::identity().det_sign_exact().unwrap(), 1);
267279
}
268280

269281
#[test]
270282
fn det_sign_exact_identity_4d() {
271-
assert_eq!(Matrix::<4>::identity().det_sign_exact(), 1);
283+
assert_eq!(Matrix::<4>::identity().det_sign_exact().unwrap(), 1);
272284
}
273285

274286
#[test]
275287
fn det_sign_exact_identity_5d() {
276-
assert_eq!(Matrix::<5>::identity().det_sign_exact(), 1);
288+
assert_eq!(Matrix::<5>::identity().det_sign_exact().unwrap(), 1);
277289
}
278290

279291
#[test]
@@ -283,35 +295,35 @@ mod tests {
283295
[4.0, 5.0, 6.0],
284296
[1.0, 2.0, 3.0], // duplicate of row 0
285297
]);
286-
assert_eq!(m.det_sign_exact(), 0);
298+
assert_eq!(m.det_sign_exact().unwrap(), 0);
287299
}
288300

289301
#[test]
290302
fn det_sign_exact_singular_linear_combination() {
291303
// Row 2 = row 0 + row 1 in exact arithmetic.
292304
let m = Matrix::<3>::from_rows([[1.0, 2.0, 3.0], [4.0, 5.0, 6.0], [5.0, 7.0, 9.0]]);
293-
assert_eq!(m.det_sign_exact(), 0);
305+
assert_eq!(m.det_sign_exact().unwrap(), 0);
294306
}
295307

296308
#[test]
297309
fn det_sign_exact_negative_det_row_swap() {
298310
// Swapping two rows of the identity negates the determinant.
299311
let m = Matrix::<3>::from_rows([[0.0, 1.0, 0.0], [1.0, 0.0, 0.0], [0.0, 0.0, 1.0]]);
300-
assert_eq!(m.det_sign_exact(), -1);
312+
assert_eq!(m.det_sign_exact().unwrap(), -1);
301313
}
302314

303315
#[test]
304316
fn det_sign_exact_negative_det_known() {
305317
// det([[1,2],[3,4]]) = 1*4 - 2*3 = -2
306318
let m = Matrix::<2>::from_rows([[1.0, 2.0], [3.0, 4.0]]);
307-
assert_eq!(m.det_sign_exact(), -1);
319+
assert_eq!(m.det_sign_exact().unwrap(), -1);
308320
}
309321

310322
#[test]
311323
fn det_sign_exact_agrees_with_det_for_spd() {
312324
// SPD matrix → positive determinant.
313325
let m = Matrix::<3>::from_rows([[4.0, 2.0, 0.0], [2.0, 5.0, 1.0], [0.0, 1.0, 3.0]]);
314-
assert_eq!(m.det_sign_exact(), 1);
326+
assert_eq!(m.det_sign_exact().unwrap(), 1);
315327
assert!(m.det(DEFAULT_PIVOT_TOL).unwrap() > 0.0);
316328
}
317329

@@ -330,7 +342,7 @@ mod tests {
330342
[7.0, 8.0, 9.0],
331343
]);
332344
// Exact: det = perturbation × (5×9 − 6×8) = perturbation × (−3) < 0.
333-
assert_eq!(m.det_sign_exact(), -1);
345+
assert_eq!(m.det_sign_exact().unwrap(), -1);
334346
}
335347

336348
/// For D ≤ 4, well-conditioned matrices should hit the fast filter
@@ -345,7 +357,7 @@ mod tests {
345357
[0.0, 0.0, 1.0, 5.0],
346358
]);
347359
// SPD tridiagonal → positive det.
348-
assert_eq!(m.det_sign_exact(), 1);
360+
assert_eq!(m.det_sign_exact().unwrap(), 1);
349361
}
350362

351363
#[test]
@@ -357,7 +369,7 @@ mod tests {
357369
[0.0, 1.0, 4.0, 1.0],
358370
[0.0, 0.0, 1.0, 5.0],
359371
]);
360-
assert_eq!(m.det_sign_exact(), -1);
372+
assert_eq!(m.det_sign_exact().unwrap(), -1);
361373
}
362374

363375
#[test]
@@ -368,38 +380,34 @@ mod tests {
368380

369381
let m = Matrix::<2>::from_rows([[tiny, 0.0], [0.0, tiny]]);
370382
// det = tiny^2 > 0
371-
assert_eq!(m.det_sign_exact(), 1);
383+
assert_eq!(m.det_sign_exact().unwrap(), 1);
372384
}
373385

374386
#[test]
375-
#[should_panic(expected = "non-finite matrix entry")]
376-
fn det_sign_exact_panics_on_nan() {
387+
fn det_sign_exact_returns_err_on_nan() {
377388
let m = Matrix::<2>::from_rows([[f64::NAN, 0.0], [0.0, 1.0]]);
378-
let _ = m.det_sign_exact();
389+
assert_eq!(m.det_sign_exact(), Err(LaError::NonFinite { col: 0 }));
379390
}
380391

381392
#[test]
382-
#[should_panic(expected = "non-finite matrix entry")]
383-
fn det_sign_exact_panics_on_infinity() {
393+
fn det_sign_exact_returns_err_on_infinity() {
384394
let m = Matrix::<2>::from_rows([[f64::INFINITY, 0.0], [0.0, 1.0]]);
385-
let _ = m.det_sign_exact();
395+
assert_eq!(m.det_sign_exact(), Err(LaError::NonFinite { col: 0 }));
386396
}
387397

388398
#[test]
389-
#[should_panic(expected = "non-finite matrix entry")]
390-
fn det_sign_exact_panics_on_nan_5x5() {
399+
fn det_sign_exact_returns_err_on_nan_5x5() {
391400
// D ≥ 5 bypasses the fast filter, exercising the bareiss_det path.
392401
let mut m = Matrix::<5>::identity();
393402
m.set(2, 3, f64::NAN);
394-
let _ = m.det_sign_exact();
403+
assert_eq!(m.det_sign_exact(), Err(LaError::NonFinite { col: 3 }));
395404
}
396405

397406
#[test]
398-
#[should_panic(expected = "non-finite matrix entry")]
399-
fn det_sign_exact_panics_on_infinity_5x5() {
407+
fn det_sign_exact_returns_err_on_infinity_5x5() {
400408
let mut m = Matrix::<5>::identity();
401409
m.set(0, 0, f64::INFINITY);
402-
let _ = m.det_sign_exact();
410+
assert_eq!(m.det_sign_exact(), Err(LaError::NonFinite { col: 0 }));
403411
}
404412

405413
#[test]
@@ -413,7 +421,7 @@ mod tests {
413421
[0.0, 0.0, 0.0, 1.0, 0.0],
414422
[0.0, 0.0, 0.0, 0.0, 1.0],
415423
]);
416-
assert_eq!(m.det_sign_exact(), -1);
424+
assert_eq!(m.det_sign_exact().unwrap(), -1);
417425
}
418426

419427
#[test]
@@ -427,7 +435,7 @@ mod tests {
427435
[0.0, 0.0, 0.0, 0.0, 1.0],
428436
]);
429437
// Two transpositions → even permutation → det = +1
430-
assert_eq!(m.det_sign_exact(), 1);
438+
assert_eq!(m.det_sign_exact().unwrap(), 1);
431439
}
432440

433441
// -----------------------------------------------------------------------
@@ -500,4 +508,16 @@ mod tests {
500508
let det = bareiss_det(&m);
501509
assert_eq!(det, BigRational::from_integer(BigInt::from(0)));
502510
}
511+
512+
#[test]
513+
fn det_sign_exact_overflow_determinant_finite_entries() {
514+
// Entries near f64::MAX are finite, but the f64 determinant overflows
515+
// to infinity. The fast filter should be skipped and Bareiss should
516+
// compute the correct positive sign.
517+
let big = f64::MAX / 2.0;
518+
assert!(big.is_finite());
519+
let m = Matrix::<3>::from_rows([[0.0, 0.0, 1.0], [big, 0.0, 1.0], [0.0, big, 1.0]]);
520+
// det = big^2 > 0
521+
assert_eq!(m.det_sign_exact().unwrap(), 1);
522+
}
503523
}

src/ldlt.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ impl<const D: usize> Ldlt<D> {
3939
for j in 0..D {
4040
let d = f.rows[j][j];
4141
if !d.is_finite() {
42-
return Err(LaError::NonFinite { pivot_col: j });
42+
return Err(LaError::NonFinite { col: j });
4343
}
4444
if d <= tol {
4545
return Err(LaError::Singular { pivot_col: j });
@@ -49,7 +49,7 @@ impl<const D: usize> Ldlt<D> {
4949
for i in (j + 1)..D {
5050
let l = f.rows[i][j] / d;
5151
if !l.is_finite() {
52-
return Err(LaError::NonFinite { pivot_col: j });
52+
return Err(LaError::NonFinite { col: j });
5353
}
5454
f.rows[i][j] = l;
5555
}
@@ -63,7 +63,7 @@ impl<const D: usize> Ldlt<D> {
6363
let l_k = f.rows[k][j];
6464
let new_val = (-l_i_d).mul_add(l_k, f.rows[i][k]);
6565
if !new_val.is_finite() {
66-
return Err(LaError::NonFinite { pivot_col: j });
66+
return Err(LaError::NonFinite { col: j });
6767
}
6868
f.rows[i][k] = new_val;
6969
}
@@ -132,7 +132,7 @@ impl<const D: usize> Ldlt<D> {
132132
sum = (-row[j]).mul_add(*x_j, sum);
133133
}
134134
if !sum.is_finite() {
135-
return Err(LaError::NonFinite { pivot_col: i });
135+
return Err(LaError::NonFinite { col: i });
136136
}
137137
x[i] = sum;
138138
}
@@ -141,15 +141,15 @@ impl<const D: usize> Ldlt<D> {
141141
for (i, x_i) in x.iter_mut().enumerate().take(D) {
142142
let diag = self.factors.rows[i][i];
143143
if !diag.is_finite() {
144-
return Err(LaError::NonFinite { pivot_col: i });
144+
return Err(LaError::NonFinite { col: i });
145145
}
146146
if diag <= self.tol {
147147
return Err(LaError::Singular { pivot_col: i });
148148
}
149149

150150
let v = *x_i / diag;
151151
if !v.is_finite() {
152-
return Err(LaError::NonFinite { pivot_col: i });
152+
return Err(LaError::NonFinite { col: i });
153153
}
154154
*x_i = v;
155155
}
@@ -162,7 +162,7 @@ impl<const D: usize> Ldlt<D> {
162162
sum = (-self.factors.rows[j][i]).mul_add(*x_j, sum);
163163
}
164164
if !sum.is_finite() {
165-
return Err(LaError::NonFinite { pivot_col: i });
165+
return Err(LaError::NonFinite { col: i });
166166
}
167167
x[i] = sum;
168168
}
@@ -331,6 +331,6 @@ mod tests {
331331
fn nonfinite_detected() {
332332
let a = Matrix::<2>::from_rows([[f64::NAN, 0.0], [0.0, 1.0]]);
333333
let err = a.ldlt(DEFAULT_SINGULAR_TOL).unwrap_err();
334-
assert_eq!(err, LaError::NonFinite { pivot_col: 0 });
334+
assert_eq!(err, LaError::NonFinite { col: 0 });
335335
}
336336
}

0 commit comments

Comments
 (0)