New System of Traits

[Coder-256]

I am working on adding complex number support to a linear algebra library. However, one major issue is that many functions rely on the Real trait despite using functions that apply to both real and complex numbers. Take, for example, the absolute value function. It simply returns a number's distance from zero. For real numbers, this will be the number itself or its additive inverse, but for complex numbers, this is also defined: it is simply the distance from the origin of the complex plane. For example, |4+3i| is equal to sqrt(4^2 + 3^2) = sqrt(16 + 9) = sqrt(25) = 5.

The issue is that many methods in the Real trait also apply to complex numbers. Now the easy fix would be to simply move those specific methods into their own trait and have Real inherit from that trait, but I think it would be better for people to create their own more abstract types (maybe fractions or 3D vectors) that implement some but not all operations.

My specific reason for this change is that I wanted to take the Euclidean norm of a matrix of complex numbers, but the library (see dimforge/nalgebra#503) relies on Real just for absolute value and addition (since the norm of a matrix is the sum of the absolute values of each element).

I have created a rough base for what I think the new traits could look like on my reorganize branch. Again, this is a very rough draft, and is definitely subject to change. For that reason, I have not implemented the traits yet, but the implementations should be nearly identical to what they currently are, but I plan to move them all into a single macro for all integers and a single macro for all floats; that way the implementation is factored out of the traits themselves and it is less redundant to account for crate options. Additionally, I have used self instead of &self in all traits so that you can implement the trait directly for types like i8 but by reference for types such as big ints.

For example, take this method in nalgebra:

pub fn norm_squared(&self) -> N {
    let mut res = N::zero();

    for i in 0..self.ncols() {
        let col = self.column(i);
        res += col.dot(&col)
    }

    res
}

Rather than requiring the numeric type to conform to Real, it really only needs N to implement addition and absolute value. That way, you can use it on your own types that implement those traits (eg. complex numbers) without also having to implement all the extraneous/incompatible methods of Real.

If you have time to look into this, please let me know what you think. Thank you!

[cuviper]

I'd really prefer to avoid breaking changes. These traits are often in the public API of other crates, which will have a large trickle-down effect to bump semver, as such downstream crates will have to bump too. That includes nalgebra -- changing that N: Real to anything else is a breaking change for them.

If/when we finally do make breaking changes here, there's a large list of possibilities in #47 to dig through. Maybe your branch already addresses some of these, but I would have to see it approached in a much more incremental way to have any hope of evaluating this.

In the short term, I think we should just consider the direct requirements for your use case. If all you really need is Abs + Add, we can add trait Abs and implement this in num-complex too.

[Coder-256]

@cuviper You make some good points. I think that you're right, it would be good to do incremental changes, but I feel very strongly that this project needs an overhaul. However, the changes I am suggesting are actually additive for the most part, and there's nothing stopping me from adding a deprecated Real trait for backwards compatibility which simply inherits from the new, refactored traits.

I completely understand that I am drafting/proposing very fundamental changes internally, which does make it tedious to evaluate. But with all that said, maybe this library is ready for an overhaul and v1.0 (or a separately maintained fork with these and other features from #47? (I am new to this library, but considering the number of breaking changes that are on hold, I think that starting work on v1.0 would be great to move the library forward). Thoughts?

[cuviper]

Frankly, you are a new contributor -- please start small and build trust before you propose an overhaul. We can add new things now to solve immediate needs, without hacking everything to pieces.

there's nothing stopping me from adding a deprecated Real trait for backwards compatibility which simply inherits from the new, refactored traits.

This way is possible to create something compatible for users of the traits, but not implementors. These traits are open to implementation by more than just the core/std types.

[Coder-256]

@cuviper I absolutely understand. I agree that it is often better to do stable, incremental changes. However, I am simply asking to discuss the possibility of a rewrite and a semantic major version. There are a few reasons I suggest this: mainly it is a chance to remove legacy/obsolete code and implement breaking changes, both of which are necessary at some point. I understand your concern that these changes would require implementors to update their code, but the great part of a major version is that it only requires implementors to update their code once or infrequently, by bundling together breaking changes. I am simply suggesting to consider if it would make sense to work on a new major version, and I do not mean to impose since I know am new to the project.

As for backwards compatibility for Real, by making it a subtrait of the new, refactored traits, it would end up the same as before (the only change is that now some methods are now also accessible through their own traits). For example, the abs(self) method is moved to the Abs trait, but then trait Real: Foo + Bar + ... is edited to trait Real: Abs + Foo + Bar + .... The Real trait is unchanged, so users and implementors would retain their current code. (although IMHO it seems better/more flexible to use less restrictive traits than Real; for example, operations that make sense on vectors, matrices, complex numbers, etc. (such as finding the mean of an array) could use Add + Div instead of Real to work with a variety of types of elements)

[cuviper]

I understand what a new major version entails. I'm not ready to tackle that yet, and even when we do, it will need to be more measured than a massive commit changing everything at once.

Your Real example would require implementors to also implement the Abs trait, even if Real is otherwise identical. That makes it a breaking change, which is fine in a new major version, but we can't pretend it's completely compatible.

[Coder-256]

@cuviper Ok I understand if you're not ready to start work on a new version, that was all I was really asking (and of course it is always important to do things carefully and incrementally). In that case, I plan to develop a separate fork. No pressure to merge any of my changes, they will be just for my personal use because my project would really benefit from some of these breaking changes that have been put on hold. Again, no pressure to merge! I think that this would be the best way to go forward because I am writing new code that would be fine with breaking changes, so I could develop my own fork separately from the main crate to be fair to users who reasonably don't want to adapt for breaking changes (which I completely understand).

As for the Real example, my mistake, I was incorrect about using supertraits (you're right that implementors would need to implement both traits that way). However, the changes still do not have to be breaking: what would work instead of supertraits is an automatic implementation of Abs for types that implement Real. For example:

trait Real: Sized {
    fn abs(self) -> Self;
    fn foo(self) -> Self;
}

impl Real for i8 {
    forward! {
        Self::abs(self) -> Self;
    }

    fn foo(self) -> Self {
        self + 5
    }
}

trait Abs {
    fn abs(self) -> Self;
}

impl<T: Real> Abs for T {
    forward! {
        Self::abs(self) -> Self;
    }
}

This way, code that implement Real can do so without any changes and automatically also conform to Abs and other new traits, and at the same time, any generic functions that use the new Abs trait will work both for implementors of Real and manual implementors of Abs.

[cuviper]

impl<T: Real> Abs for T

Yes, it could be implemented this way, and then it would not be a breaking change for nalgebra to change their T: Real code to T: Abs + Add (+ any other constraints already covered by Real).

One caveat is that Real requires std for the transcendental methods. Crate features have to be additive, and adding a blanket impl for an existing trait is a breaking change. Thus, the Abs trait will have to require std too -- it can exist with the blanket impl or not at all. That's a little sad, since Abs shouldn't really need std, but this is a compatibility trade-off.

[Coder-256]

Either way, I have decided that at least for my project, it would be worth using an updated version of the library with these breaking features, because since I am starting a new project I do not rely on old APIs. This is clearly not the case for everyone, so that's why I have created my own fork and started working on my changes. It's up to you what to do with rust-num and I will maintain my own fork separately so that I don't force breaking changes on everyone. If you're interested, here is my fork (and my changes are in the reorganize branch). I think it still might be possible to merge these changes in a backwards-compatible way and create a compatible Real trait, but I have not done so yet in my fork, because I do not need it personally.

Overall, I'm planning to separately maintain my own fork, feel free to look at my changes but feel no pressure to merge them back into this repo, and you can close this issue if you want because I'm OK with this solution.