rust-num / num-complex

Complex numbers for Rust
Apache License 2.0
232 stars 50 forks source link

Implement Ord for Complex #106

Closed annoybot closed 6 months ago

annoybot commented 2 years ago

I've got to write a lot of unit tests for functions that will be producing lists of complex numbers.

They come out in an arbitrary order so I was hoping to stably sort the expected results and the actual results and then to run a loop over both asserting them to be equal.

I tried doing this with sort_by() but it is indeed unstable, and I can't rely on it for these purposes.

Would it be possible to implement Ord for Complex so that stable sort can be used?

cuviper commented 2 years ago

It does not make mathematical sense, as discussed in #107, but you should still be able to customize your own sorting.

I tried doing this with sort_by() but it is indeed unstable, and I can't rely on it for these purposes.

I'm not sure what you mean here. It's stable as far as the API is concerned, and also stable in the sorting sense where equal items are kept in their original order relative to each other. For types like Complex64 = Complex<f64>, equal values are completely indistinguishable, so sort_unstable_by should work fine too.

So if you want to sort Complex64 values by re, then im, that's something like:

    slice.sort_unstable_by(|x, y| (x.re, x.im).partial_cmp(&(y.re, y.im)).unwrap());

If you want to permit NaN as well, there's a total_cmp coming in Rust 1.62 for both f32 and f64.

cuviper commented 2 years ago

For types like Complex64 = Complex<f64>, equal values are completely indistinguishable,

Oh sorry, that's not actually true with +0.0 and -0.0, so if you care about that you should use sort_by to keep it stable, or a total order to distinguish those signs.

annoybot commented 2 years ago

I managed to get this implemented as described here but there was an interesting wrinkle that made it practically useless for my purposes.

I have some code that I'm porting from python where they have a lot of asserts that look like this:

assert_allclose(sort(bla), sort([-2j, -9j, 0, +9j, +2j]))

Basically it sorts the results and the expected results then compares that they are all close. Great.

I Implement a function which sorted the Complex lexicographically, which is to say that it sorted first by re and then by im.

So if you have a list of complex numbers like this: [0+3i, 0+1i, 0+2i] then the sort would indeed sort them, with the imaginary parts ascending, and I tried to structure my asserts in a similar way.

But in practical terms, because of floating point error, values near zero would flip sign, from -ϵ to + ϵ.

So in actual use, my results would look like this say: [-ϵ+3i, ϵ+1i, ϵ+2i]

This made the feature useless to me now because the sort order, while still deterministic, was hard to predict unless I just ran the code.

So I couldn't rely on it in the end.

cuviper commented 2 years ago

Maybe it would work better for you to sort by polar (r, theta)? I think that would get you near enough to let fuzzy "close" equality work on the cartesian coordinates. But that could still fail if you have expected values of similar magnitude, and one angle is near ±π.

annoybot commented 2 years ago

Yeah, I think an optional ϵ parameter to the sort would be helpful.