code review 227041: Spec for complex numbers

Spec for complex numbers

[r, 2010-03-03 20:27:38]

Hello rsc, ken2, gri, iant (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[rsc, 2010-03-03 20:55:24]

http://codereview.appspot.com/227041/diff/1/2
File doc/go_spec.html (right):

http://codereview.appspot.com/227041/diff/1/2#newcode324
doc/go_spec.html:324: imaginary_lit = (decimals | float_lit ) "i" .
space after (

http://codereview.appspot.com/227041/diff/1/2#newcode514
doc/go_spec.html:514: and <code>real</code> and <code>imag</code> applied to a
complex constant.
and cmplx applied to numeric constants.

http://codereview.appspot.com/227041/diff/1/2#newcode3262
doc/go_spec.html:3262: than conversion of an imaginary constant to a complex
type.
I think the mention of constant can be dropped.
This section doesn't talk about the fact that
int(1.5) is invalid either.

http://codereview.appspot.com/227041/diff/1/2#newcode3377
doc/go_spec.html:3377: Imaginary literals have complex type (with zero real
part)
I agree that this text should be here,
but the corresponding sentence for floating and integer
constants is missing.  (It's in the #Assignments section,
in the last paragraph before the #If_statements section.)
Maybe move that text up?

http://codereview.appspot.com/227041/diff/1/2#newcode4488
doc/go_spec.html:4488: var c64 = cmplx(math.Cos(math.Pi/2), math.Sin(math.Pi/2))
 // has type complex64
has type complex128 - Cos and Sin return float64.

http://codereview.appspot.com/227041/diff/1/2#newcode4490
doc/go_spec.html:4490: var rl = real(c64)  // type float32
float64

http://codereview.appspot.com/227041/diff/1/2#newcode4923
doc/go_spec.html:4923: <li><span class="alert">The implementation of complex
numbers is incomplete. Gccgo does not support them.</span></li>
I think you could stop at "is incomplete."
Even 8g doesn't support them.

[gri, 2010-03-03 21:15:46]

http://codereview.appspot.com/227041/diff/1/2
File doc/go_spec.html (right):

http://codereview.appspot.com/227041/diff/1/2#newcode320
doc/go_spec.html:320: or decimal integer followed
... followed by the lower-case letter ...

should be enough. The syntax speaks for itself, and we don't say "immediately,
without whitespace" when we talk about the exponent in floats.

http://codereview.appspot.com/227041/diff/1/2#newcode643
doc/go_spec.html:643: complex64   the set of all complex numbers with float32
real and imaginary parts
I'd probably call this complex32 and complex64.

http://codereview.appspot.com/227041/diff/1/2#newcode4452
doc/go_spec.html:4452: <h3 id="Complex_numbers">Complex numbers</h3>
Support for complex numbers?

(The other sections are named after the functionality, and not the types
involved).

http://codereview.appspot.com/227041/diff/1/2#newcode4457
doc/go_spec.html:4457: value from a floating-point real and imaginary part,
while
it can also be an untyped constants, can't it? it needs to be
assignment-compatible, but as it stands, it sounds misleading

http://codereview.appspot.com/227041/diff/1/2#newcode4471
doc/go_spec.html:4471: floating-point type and the return type is the complex
type
same issue here. I can write cmplx(2, 3) and neither 2 or 3 are of type float

http://codereview.appspot.com/227041/diff/1/2#newcode4482
doc/go_spec.html:4482: If the operands of these functions are constants, the
return
s/the/all/ ?

http://codereview.appspot.com/227041/diff/1/2#newcode4486
doc/go_spec.html:4486: <pre class="grammar">
examples don't have class "grammar" elsewhere

[iant, 2010-03-03 21:44:14]

FYI

http://codereview.appspot.com/227041/diff/1/2
File doc/go_spec.html (right):

http://codereview.appspot.com/227041/diff/1/2#newcode3377
doc/go_spec.html:3377: Imaginary literals have complex type (with zero real
part)
It's somewhat ambiguous to say "have complex type".  Do you mean the specific
type "complex", or do you mean "complex, complex64, or complex128", or do you
mean "untyped complex?"

http://codereview.appspot.com/227041/diff/1/2#newcode4455
doc/go_spec.html:4455: Several functions assemble and disassemble complex
numbers.
s/Several/Three/ ?

(I don't think of three as several)

[r2, 2010-03-03 22:15:12]

On Mar 3, 2010, at 1:44 PM, iant@golang.org wrote:

> FYI
>
>
> http://codereview.appspot.com/227041/diff/1/2
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/227041/diff/1/2#newcode3377
> doc/go_spec.html:3377: Imaginary literals have complex type (with zero
> real part)
> It's somewhat ambiguous to say "have complex type".  Do you mean the
> specific type "complex", or do you mean "complex, complex64, or
> complex128", or do you mean "untyped complex?"
>
> http://codereview.appspot.com/227041/diff/1/2#newcode4455
> doc/go_spec.html:4455: Several functions assemble and disassemble
> complex numbers.
> s/Several/Three/ ?
>
> (I don't think of three as several)

i think of it as "three or more" but sure, let's be precise

>
> http://codereview.appspot.com/227041/show

[r2, 2010-03-03 22:16:28]

On Mar 3, 2010, at 1:15 PM, gri@golang.org wrote:

> I'd probably call this complex32 and complex64.

me too. it just doesn't feel right that a complex64 has the same  
precision as a float32. how much do we really care about compatibility  
with other worlds?

-rob

[r2, 2010-03-03 22:30:55]

On Mar 3, 2010, at 12:55 PM, rsc@golang.org wrote:

>
> http://codereview.appspot.com/227041/diff/1/2
> File doc/go_spec.html (right):
>
> http://codereview.appspot.com/227041/diff/1/2#newcode324
> doc/go_spec.html:324: imaginary_lit = (decimals | float_lit ) "i" .
> space after (
>
> http://codereview.appspot.com/227041/diff/1/2#newcode514
> doc/go_spec.html:514: and <code>real</code> and <code>imag</code>
> applied to a complex constant.
> and cmplx applied to numeric constants.
>
> http://codereview.appspot.com/227041/diff/1/2#newcode3262
> doc/go_spec.html:3262: than conversion of an imaginary constant to a
> complex type.
> I think the mention of constant can be dropped.
> This section doesn't talk about the fact that
> int(1.5) is invalid either.
>
> http://codereview.appspot.com/227041/diff/1/2#newcode3377
> doc/go_spec.html:3377: Imaginary literals have complex type (with zero
> real part)
> I agree that this text should be here,
> but the corresponding sentence for floating and integer
> constants is missing.  (It's in the #Assignments section,
> in the last paragraph before the #If_statements section.)
> Maybe move that text up?

this discussion is about "complex constants", not assignment. i think  
it's fine.
i've added a TODO at the top to address the constant promotion rules  
in general.

[r, 2010-03-03 22:31:03]

Hello rsc, ken2, gri, iant (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2010-03-03 23:16:43]

I agree that complex64 and complex128 are unexpected,
but complex32 and complex64 are imprecise too.
Either way it's not clear what complex64 means until
you learn which convention we chose.

The arguments I see for the current names are:
  * they match the rest of the numeric types
  * a floating point number has two pieces too,
    and yet we name it by its aggregate size - float64 not float53.
  * they match NumPy and (more compelling to me) Mathematica.
  * draw already uses RGBA32 not RGBA8.

The argument against is:
  * they're float32 underneath, so complex32.

I think it's mostly a wash, but I'm still a bit in favor of
keeping the current names.

Russ

[rsc, 2010-03-04 01:47:37]

LGTM

http://codereview.appspot.com/227041/diff/1004/1005
File doc/go_spec.html (right):

http://codereview.appspot.com/227041/diff/1004/1005#newcode3381
doc/go_spec.html:3381: operations with integer constants and floating-point
constants; the
with untyped

On next line, result is always untyped, no?
e.g. "the result is an untyped complex constant."

[r2, 2010-03-04 04:31:04]

On Mar 3, 2010, at 3:16 PM, Russ Cox wrote:

> I agree that complex64 and complex128 are unexpected,
> but complex32 and complex64 are imprecise too.
> Either way it's not clear what complex64 means until
> you learn which convention we chose.
> 
> The arguments I see for the current names are:
>  * they match the rest of the numeric types
>  * a floating point number has two pieces too,
>    and yet we name it by its aggregate size - float64 not float53.
>  * they match NumPy and (more compelling to me) Mathematica.
>  * draw already uses RGBA32 not RGBA8.
> 
> The argument against is:
>  * they're float32 underneath, so complex32.

that's oversimplifying. it really bothers me a lot that a zero-imaginary-part
complex64 is exactly a float32.  they mean the same thing but the names are off
by a factor of two.

> I think it's mostly a wash, but I'm still a bit in favor of
> keeping the current names.
> 
> Russ

i understand and i'm pretty sure we'll go this way but i want to go on record
saying i dislike the names. none of your arguments in favor carries much weight,
to me. the argument against does.

on issues of naming there is little precedent in go for following the lead from
other languages when entering new territory.

-rob

[r2, 2010-03-04 04:38:18]

names aside, anyone else have comments?

[gri, 2010-03-04 05:22:29]

Looks mostly good, but I think the comparison section needs editing. There is no
linear ordering defined on complex.

http://codereview.appspot.com/227041/diff/1004/1005
File doc/go_spec.html (right):

http://codereview.appspot.com/227041/diff/1004/1005#newcode2944
doc/go_spec.html:2944: All other comparison operators apply only to numeric and
string values.
For complex types, only == and != are defined.

http://codereview.appspot.com/227041/diff/1004/1005#newcode3385
doc/go_spec.html:3385: literals or constants derived from them, or calls of the
is "or constants derived from them" needed?

http://codereview.appspot.com/227041/diff/1004/1005#newcode4481
doc/go_spec.html:4481: parts, of corresponding type, from the complex value.
Instead of saying that explicitly, how about:

For a complex value z, the following relationship holds:

z == cmplx(real(z), imag(z))

http://codereview.appspot.com/227041/diff/1004/1005#newcode4493
doc/go_spec.html:4493: var c64 = cmplx(x, -x)  // has type complex64
perhaps make one of the x a constant, say 5

[r, 2010-03-04 06:05:23]

PTAL

http://codereview.appspot.com/227041/diff/1004/1005
File doc/go_spec.html (right):

http://codereview.appspot.com/227041/diff/1004/1005#newcode3385
doc/go_spec.html:3385: literals or constants derived from them, or calls of the
On 2010/03/04 05:22:29, gri wrote:
> is "or constants derived from them" needed?

i think so.

const i = 1i
const z = 1+i

the expression on the right of the second line is a complex constant but
contains no imaginary literal.

[cw, 2010-03-04 06:32:00]

On 2010/03/04 04:38:18, r2 wrote:
> names aside, anyone else have comments?

why is this even going into the spec at this stage?

can it not be left as an implementation that's not part of the formal
specification until people have more time to mull over the details?

many people won't have watched this thread or the earlier commits and will only
be exposed to this once the functionality it's fairly visible (ie. things like
fmt.Print* supported) and so to me it makes sense to implement this and then
wait for feedback

[r, 2010-03-04 07:06:31]

Hello rsc, ken2, gri, iant, cw (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc, 2010-03-04 07:25:47]

> why is this even going into the spec at this stage?
>
> can it not be left as an implementation that's not part of the formal
> specification until people have more time to mull over the details?

That's how it works: you write down the rules so that people
can see what they are and comment on them.  Writing down
the current rules doesn't prevent changes later, and it does
make sure that all the developers and both compiler suites
agree on what the implementation should be.  This is admittedly
different from many other popular languages today, where the
implementation is the definition and documentation, if any,
always lags behind.  But I assert that Go's approach is clearer.
You can't mull over the details unless you know what they are.

Russ

[rsc, 2010-03-04 17:49:05]

LGTM

[gri, 2010-03-04 18:57:24]

LGTM

http://codereview.appspot.com/227041/diff/2004/3007
File doc/go_spec.html (right):

http://codereview.appspot.com/227041/diff/2004/3007#newcode2942
doc/go_spec.html:2942: The operators <code>==</code> and <code>!=</code> apply,
at least in some cases,
Do we need "at least in some cases"? We have the exception mentioned explicitly.

[iant, 2010-03-04 19:12:09]

LGTM

[r, 2010-03-04 20:35:25]

*** Submitted as http://code.google.com/p/go/source/detail?r=d761fd7a5f7f ***

Spec for complex numbers

R=rsc, ken2, gri, iant
CC=cw, golang-dev
http://codereview.appspot.com/227041