Iron out semantics of Complex(length:phase:)

[apple]

Iron out semantics of Complex(length:phase:)

[stephentyrone]

So looking at this, I realized that I also haven't accounted for length < 0. We can make sense of this mathematically, but I'm not sure that we actually want to do so from a UX perspective. I'm somewhat tempted to make this non-failable, but with preconditions that length >= 0 (hence non-nan, non-negative) and phase.isFinite, because then the rules and behavior are trivially explainable to users (and if experts want some specific other behavior, it's easy to implement).

The only real downside I see is that you can't round-trip zero and infinity to polar and back this way, which is slightly unfortunate.

[NevinBR]

We could enable round-tripping by slightly changing the preconditions to require finite phase unless length is 0 or infinite:

length >= 0
phase.isFinite || (length == 0 || !length.isFinite)

[stephentyrone]

.nan, .nan is not a representation for invalid results; it's treated as another infinity.

This is somewhat surprising. The advantage of this choice is that it lets us use the obvious expression for multiplication, (rr-ii, ri+ri) rather than having to detect infinities and handle them specially (if you try to distinguish infinity and nan, then you have to deal with the fact that (inf,0) * (0, i) would give you (nan, inf), which ... is that a nan or an infinity? This is a mess for C and C++,it results in everyone who cares about performance using their own multiplication instead of the language-provided one).

There are also downsides to this approach, but it pushes some of the weirdness out of basic arithmetic and onto less-frequently-used operations. I'm open to changing it, but there's a definite tension.

[NevinBR]

.nan, .nan is not a representation for invalid results; it's treated as another infinity.

I understand that it is treated as an infinity, yet I maintain that it is a representation of invalid results. Or at least, Complex.infinity in general is currently used to represent invalid results:

.infinity - .infinity is undefined, but represented by .infinity
.infinity / .infinity is undefined, but represented by .infinity
(0, 0) / (0, 0) is undefined, but represented by…wait, that’s (0, 0)? Is this intentional? I would expect .infinity.

[stephentyrone]

(0, 0) / (0, 0) is undefined, but represented by…wait, that’s (0, 0)? Is this intentional? I would expect .infinity.

Yeah, that's a bug (#54)

[stephentyrone]

Yeah, the only downside is that it makes the behavior slightly less easy to explain. I'll sleep on it.

[NevinBR]

Do we need an explicit precondition at all? We could do something like this:

init(length: RealType, phase: RealType) {
  if length == 0 {
    self = .zero
  } else if !length.isFinite {
    self = .infinity
  } else {
    self.init(length * .cos(phase), length * .sin(phase))
  }
}

This leaves negative length behaving as it currently does. We don’t necessarily have to document that negative lengths are supported, but I see no benefit to trapping on them. This behavior is at least sensible and doesn’t entail any extra runtime checks.

[stephentyrone]

Yes, I think if someone asks for a Complex number with finite length but non-finite phase, that's a programming error we should detect via a precondition. I'm less worried about negative lengths, since there is a perfectly reasonable answer in that case, it's just maybe not the answer they expect.

[NevinBR]

Sure, that makes sense.

[stephentyrone]

Nevin, please close unless you think further changes are needed.