Summing Vec<Complex<T>>: difference between core::num::Zero and num::traits::Zero

[SuperFluffy]

I wanted to sum a Vec<Complex<T>> using std::iter::Iterator::sum(), which has the signature:

fn sum<S = Self::Item>(self) -> S where S: Add<Self::Item, Output=S> + Zero { ... }

It turns out that the trait core::num::Zero referred to in the signature is different from num::traits::Zero defined in rust-num. You get an error like:

error: the trait `core::num::Zero` is not implemented for the type `num::complex::Complex<f64>` [E0277]

I think it is quite unfortunate that these two traits are not the same. They should be.

A MWE to show the error would be:

use num::complex::Complex;
use num::traits::Zero;

fn main () {
    let cvec = vec![Complex::<f64>::new(1.0,1.0); 10];
    let s = cvec.iter().sum();
}

[cuviper]

I don't think it would be enough to just give them identical signatures -- they are still distinct traits from different sources. core::num::Zero is unstable, otherwise we could implement it for Complex, and even make num::traits::Zero inherit it. Maybe add those impls hidden under a nightly feature?

[PeterHatch]

I ran into this issue, and made changes that fix it for me, but aren't ready to merge yet - PeterHatch@59c70e9

Most importantly it needs to still work when not using nightly; I'm not sure how to detect that - I'd appreciate pointers, and may look into it more. And the same thing should be done for One.

[cuviper]

Perhaps we should just provide our own num-flavored sum and product.

[PeterHatch]

How would that sum and product be provided and used? I can't think of a way a library could do it that would be as convenient for users as the nightly's version of methods directly on Iterator.

Also, would we want the num-flavored versions to stick around after the standard library versions are finalized? Not sure how this project is handling backwards-compatibility, but if there's a significant chance we'd want them dropped in the future that seems like an argument against adding them...

Um, and I now notice that the standard library issue for stabilizing Zero/One/iter_arith (rust-lang/rust#27739) is in the final comment period, so probably any decisions here should wait until that's decided. (Though options here seem relevant to mention there.)

[cuviper]

We would provide them through our own trait, say NumIterator, which we'd implement for any Iterator. If someone writes use num::NumIterator; they can call any method therein. These specific methods would have the Zero/One/Add/Mul constraints as needed, just like the unstable sum and product do now. I haven't tried, but I think there will be call ambiguity unless we name ours differently -- maybe summ and prod? Or num_sum and num_product?

As for stabilization -- num is still compatible with Rust 1.0, and I consider it a breaking change to raise that minimum. If those std versions do become stable, then eventually we may move up to whatever version that is, and then the equivalent num traits can go away or just be pub use std..., reexported. Until that day, since there's no way to cfg rust versions, the num traits will just have to exist in parallel.

[PeterHatch]

Oh, right, that NumIterator plan would work. Glad I was overlooking something. 😅

For stabilization, I ran across https://github.com/rust-lang/rfcs/blob/master/text/1105-api-evolution.md which suggests using cargo features - to quote that section:

Crates

Major change: going from stable to nightly

Changing a crate from working on stable Rust to requiring a nightly is considered a breaking change. That includes using #[feature] directly, or using a dependency that does so. Crate authors should consider using Cargo "features" for their crate to make such use opt-in.

Minor change: altering the use of Cargo features

Cargo packages can provide opt-in features, which enable #[cfg] options. When a common dependency is compiled, it is done so with the union of all features opted into by any packages using the dependency. That means that adding or removing a feature could technically break other, unrelated code.

However, such breakage always represents a bug: packages are supposed to support any combination of features, and if another client of the package depends on a given feature, that client should specify the opt-in themselves.

I think using a feature to enable extending std::num::Zero and std::num::One would work. Except that if the feature was turned on, that would break any other library that had a type that extended num's Zero or One without extending the std version, and even if they had their own flag to fix the issue I'm not sure they could detect if num had the feature turned on. (Maybe libraries with a type that implement num's Zero or One type are uncommon enough this would be okay?)

Also, I don't think it will be as simple as num's types just going away or reexporting the std versions. Right now the std version doesn't include the is_zero() function, and the linked stabilization thread includes the argument that:

Since we’ve already decided to reduce the scope of std::num, more features like in-place zeroing or dealing with precision probably belong in the num crate instead.

It seems quite possible to have num include types that extend the std types, but I'd be more comfortable with other people expecting that to happen if you were also expecting it, and there was a concrete plan for how it would be done.

[cuviper]

OK, sure, we can plan on keeping num extensions like is_zero().

[bluss]

std Zero is now gone.