simba icon indicating copy to clipboard operation
simba copied to clipboard

Allow direct conversions from f32 to RealField or ComplexField

Open patowen opened this issue 1 year ago • 0 comments

Fixes #54. This should help avoid surprises for users of crates like nalgebra when, for instance, casting f32 matrices to RealField matrices.

One concern that is likely to block a merge of this PR is that anyone who implements RealField and ComplexField would need to implement an additional trait (SupersetOf<f32>). Given that both f32 and f64 have are in frequent use, this might be better long-term, but it would require a major version bump.

I'm not familiar enough with Rust to know whether it's possible to handle this in a backwards-compatible way. As far as I can tell, based on https://github.com/rust-lang/rust/issues/31844, it's not possible to create a default implementation. If I were to write the following

impl<T> SubsetOf<f32> for T where T: ComplexField {
    fn to_superset(&self) -> f32 {
        f64::from_subset(self) as f32
    }

    fn from_superset_unchecked(element: &f32) -> Self {
        f64::from_superset_unchecked(element) as f32
    }

    fn is_in_subset(element: &f32) -> bool {
        f64::is_in_subset(element)
    }
}

it would not compile because it conflicts with the more specific macro in subset.rs that effectively run impl SubsetOf<f32> for f32 and impl SubsetOf<32> for f64.

The right answer might be to keep serde the way it is, although it may be good to document against potential pitfalls that could create. For instance, it hampers nalgebra's cast method for anyone trying to cast an f32 matrix to a RealField matrix. However, if RealField is mostly meant to be used internally by dimforge crates, this might be a non-issue, in which case this PR should almost definitely be closed without merging.

patowen avatar Feb 25 '24 05:02 patowen