Fix Issue 11787 - std.complex should have a separate Imaginary type.

[WebDrake]

The rationale for a separate imaginary type has been laid out in Kahan & Thomas, "Augmenting a programming language with complex arithmetic", http://www.eecs.berkeley.edu/Pubs/TechRpts/1992/CSD-92-667.pdf

In total this patch implements:

• The Imaginary type;
• imaginary-numeric operations via Imaginary.opBinary, and numeric-imaginary operations via Imaginary.opBinaryRight
• complex-imaginary operations via Complex.opBinary, and imaginary-complex operations via Complex.opBinaryRight;
• abs and arg implementations to support Imaginary.

A broader range of imaginary-supporting functions (e.g. sin, cos, etc.) are not yet implemented; this will be done conditional on acceptance of the general design.

Issue report: https://d.puremagic.com/issues/show_bug.cgi?id=11787

[andralex]

push these down to where they're used :o)

[WebDrake]

Ah, I see I'm the guinea pig? :-)

[andralex]

From my painful experience it's much easier to do this in the beginning rather than later...

[WebDrake]

I agree, good to be moving on it already!

[WebDrake]

@andralex -- thanks for the detailed feedback. The new patches provide partial fixes for the issues raised -- more to follow. Some notes:

• the moving of imports to internal-as-possible locations was quite easy. std.math and std.traits needed to stay top-level: std.traits for obvious reasons, std.math because the complex-based abs functions were dependent on the Complex and Imaginary types and conversely operations on the Complex and Imaginary types were dependent on the complex-based abs.
• The nan case in arg has been dealt with.
• Imaginary's Complex-based constructors, opAssign and opEquals have been removed.
• Regarding complex-imaginary and imaginary-numeric opEquals -- I'm not ignoring your requests to remove, they'll be stripped out in patches to arrive later today -- but just as a trial run, I've put in the Complex opEquals imaginary and preserved a modified Imaginary opEquals numeric. I just want to see what the test suite makes of it :-)

I'll follow up later today with patches addressing the opBinary and opBinaryRight issues and stripping out the remaining problematic opEquals.

[WebDrake]

the moving of imports to internal-as-possible locations was quite easy

Aack. I just realized I didn't include a commit for that -- I must have done it, stashed it, forgotten about it. Anyway, the description stands -- std.math and std.traits need to be top-level, all else can be relegated to lower down.

[WebDrake]

Looks like similar unittest failures as those encountered with pull request #1740. I'll see if I can fix those.

[WebDrake]

OK, status update:

• imports have been relocated :-)
• nan case in arg fixed
• Imaginary's complex-based constructor, opAssign and opEquals have been removed
• unittests seem to be working this time (FP issues worked around...)
• Complex.opBinary that forwards to opOpAssign: I accept the point but would like to make any changes in a separate pull request. The current design reflects what's already there for complex op complex and complex op numeric, it should either all be changed at once or kept as is; and since refactoring that code will involve code duplication and needs specific care and attention, I'd rather do it separately than bundle it with other stuff.
• Support for integer-based complex/imaginary types: again, I think this is best done as a project in its own right. The current imaginary() helper function reflects what's present for complex().
• complex == imaginary and imaginary == numeric opEquals: sorry but I want to dig my heels in here. 2 reasons: (i) it doesn't make sense to allow complex == numeric but not complex == imaginary; (ii) perhaps more importantly, it breaks feature parity with the built-in complex and imaginary types. E.g.:

    real re = 0.0;
    ireal im = 0.0i;
    creal z = 0.0 + 0.0i;
    writeln(z == re);
    writeln(z == im);
    writeln(im == re);

... will output three true statements. So, it potentially breaks the ability to replace creal, ireal, etc. with Complex and Imaginary, if we don't allow these equality comparisons.

You can overrule me :-) -- but I wanted to make sure that these factors were at least acknowledged before a final decision.

[andralex]

My understanding is that allowing complex == imaginary may interfere with the rationale of introducing Imaginary in the first place, which was (in my understanding) as follows:

"A complex number with a 0.0 real part is often a complex number with a real part so small, it cannot be represented with the floating point underlying that number. In contrast, an Imaginary object is a complex number with an actual mathematical zero for the real part."

That said, it's sensible to allow comparison between a number with no real part and one with a really small real part. I'm deferring this to @donc and others who know more about complex numbers. Since Don has been silent as of late he may be vacationing so let's let this marinate for a short while. Please ping when you see him around.

[yebblies]

@andralex If the built-in complex supports it, so should the library type. Note also that (-0.0 == +0.0) == true.

[WebDrake]

I'd say that the rationale for introducing an Imaginary is that a complex number with real part 0.0 introduces extra error (and in some cases spurious results) for otherwise legitimate calculations. So, you want a way to avoid that in circumstances when you know for certain that the real part is exactly 0.

Agree that we should wait for @donc -- I'm keen to get his feedback, even if it turns out to be "Take this monstrosity away!!" ;-)

One point he raised on the mailing list was that it might be possible to do without an imaginary type per se and just have a representation of i. I've been considering that that, but it's not obvious to me how to do it in a "nice" way (or what symbol to use if we do that).

[monarchdodra]

My understanding is that allowing complex == imaginary may interfere with the rationale of introducing Imaginary in the first place, which was (in my understanding) as follows:

"A complex number with a 0.0 real part is often a complex number with a real part so small, it cannot be represented with the floating point underlying that number. In contrast, an Imaginary object is a complex number with an actual mathematical zero for the real part."

Imaginary is the counterpart to the "natural" doubles. If "Complex == double" is legal, then so should "Complex == Imaginary".

Your argument can be written the other way:
"A complex number with a 0.0 imaginary part is often a complex number with an imaginary part so small, it cannot be represented with the floating point underlying that number. In contrast, an natural object (double) is a complex number with an actual mathematical zero for the imaginary part."

I think not having "Complex == Imaginary" would just be inconsistent.

[quickfur]

ping
Any decision on this PR? Should we move ahead, or close it pending further discussion?

[WebDrake]

IIRC we were basically waiting for feedback. Sorry for not pushing more about that.

[mihails-strasuns]

This waits for @donc - @WebDrake I suggest you to ask him personally ;)

[WebDrake]

Yea, I should stop dropping subtle hints and nudge him properly ;-)

[quickfur]

ping @donc @WebDrake

[quickfur]

ping @donc @WebDrake
Looks like this needs a rebase.

[quickfur]

ping
Are we going anywhere with this, or should we just close it for now?

[quickfur]

Merge conflicts: rebase please.

[mihails-strasuns]

I have talked with @WebDrake , he is unlikely to have any time to get to Phobos hacking soon. Will close his pull requests for now, he will reopen them when ready.