dimforge / dimforge/nalgebra

Poor choice of default tolerance in SVD computation

Open
#1,563 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
4.8k
Forks
565
PR merge metrics
No merged PRs in 30d

Description

Currently, the tolerance in the SVD defaults to a small multiple of `approx::AbsDiffEq::default_epsilon()`. See:

https://github.com/dimforge/nalgebra/blob/d055f22/src/linalg/svd.rs#L98

The problem is that this parameter is badly and confusingly named (see [approx discussion]): `default_epsilon()` is not related to the machine epsilon, i.e., it is not the relevant *relative* precision, but it relates to a default tolerance in the *absolute* difference. The proper method is instead, the again not terribly well-named, [`approx::RelativeEq::default_max_relative()`].

What makes matters worse is that there is usually no sensible default for absolute tolerance, as it depends on the scaling of your problem, which is why most float comparisons sets it to zero. However, in their default implementation for `f32` and `f64`, `approx` fills in machine epsilon for both default absolute and relative tolerance, thereby further blurring the distinction between the two. We instead -- properly -- set it to zero in the xprec package, which then breaks the SVD here.

[approx discussion]: https://github.com/brendanzab/approx/issues/8#issuecomment-296958843
[`approx::RelativeEq::default_max_relative()`]: https://docs.rs/approx/latest/approx/trait.RelativeEq.html#tymethod.default_max_relative

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.