Re: [numbers] Complex missing some C++ standards

[Alex Herbert]

On Mon, 30 Dec 2019, 02:23 Gilles Sadowski,  wrote:

> Hi.
>
> Le dim. 29 déc. 2019 à 23:25, Alex Herbert  a
> écrit :
> >
> > I’ve dropped the static equals methods and reciprocal and pushed the
> updated class with MathJax.
> >
> > I put MathJax in whenever possible. This may be a bit too much. The
> rendered javadoc looks good but the javadoc rendered by my IDE without
> MathJax support can be very unreadable.
>
> One cannot have one's cake and eat it too.
>
> >
> > Have a look at the built javadocs and through an IDE and let me know
> your opinions on the current usage.
>
> I think that it's fine, when looking with a pager.
> I don't use an IDE. :-}
>
> >
> > It may be better to drop some use of MathJax such as \( z \) for {@code
> z} which would make the code more readable in an IDE when programming.
>
> Sure. [I seem to recall having mentioned that such usage of {@code} was
> fine.]
>
> > I’ve not explicitly laid out the latex equations for unparsed
> readability so some improvements could be made. However some equations are
> multi-line which gets wrapped to a single line if pure HTML without
> MathJax. For example see atanh and acos. I do not know how to lay this out
> to make it readable without MathJax.
>
> No need IMO.
>
> > So perhaps a note in the class javadoc that the use of MathJax is
> required to read the formatted equations.
>
> This a general comment (i.e. valid for all classes in this component).
> But isn't MathJax active (when browsing the "Commons" site?
>

Yes. I think I'll have another look over it. Maybe separate all the
equations into their own paragraphs. Thus if they don't render they will be
easy to ignore.

I think small latex inline is still readable so I'll leave it as the
rendered javadoc is nicer.


> > On another c++ note the documentation for the C99 functions on C++
> reference lists the special return values, e.g. [1]. This is similar to the
> special return values listed in java.util.Math for functions, e.g. [2]. I
> think it would be good to add these to the javadoc. It should be a simple
> cut and reformat from the ISO C99 Annex G that the class has been tested
> against.
>
> How about including a link to the respective C++ doc pages?
>

I considered that. But having to jump about between websites to get the
information is not user friendly. I'll try out adding them to the javadoc.
It should just be about 10 list elements per C99 method so it's not that
much.


> Regards,
> Gilles
>
> >
> > [1] https://en.cppreference.com/w/c/numeric/complex/cacos <
> https://en.cppreference.com/w/c/numeric/complex/cacos>
> > [2]
> https://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#atan2-double-double-
> <
> https://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#atan2-double-double-
> >
> >
> > Alex
> >
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>

Re: [numbers] Complex missing some C++ standards

[Gilles Sadowski]

Hi.

Le dim. 29 déc. 2019 à 23:25, Alex Herbert  a écrit :
>
> I’ve dropped the static equals methods and reciprocal and pushed the updated 
> class with MathJax.
>
> I put MathJax in whenever possible. This may be a bit too much. The rendered 
> javadoc looks good but the javadoc rendered by my IDE without MathJax support 
> can be very unreadable.

One cannot have one's cake and eat it too.

>
> Have a look at the built javadocs and through an IDE and let me know your 
> opinions on the current usage.

I think that it's fine, when looking with a pager.
I don't use an IDE. :-}

>
> It may be better to drop some use of MathJax such as \( z \) for {@code z} 
> which would make the code more readable in an IDE when programming.

Sure. [I seem to recall having mentioned that such usage of {@code} was fine.]

> I’ve not explicitly laid out the latex equations for unparsed readability so 
> some improvements could be made. However some equations are multi-line which 
> gets wrapped to a single line if pure HTML without MathJax. For example see 
> atanh and acos. I do not know how to lay this out to make it readable without 
> MathJax.

No need IMO.

> So perhaps a note in the class javadoc that the use of MathJax is required to 
> read the formatted equations.

This a general comment (i.e. valid for all classes in this component).
But isn't MathJax active (when browsing the "Commons" site?

> On another c++ note the documentation for the C99 functions on C++ reference 
> lists the special return values, e.g. [1]. This is similar to the special 
> return values listed in java.util.Math for functions, e.g. [2]. I think it 
> would be good to add these to the javadoc. It should be a simple cut and 
> reformat from the ISO C99 Annex G that the class has been tested against.

How about including a link to the respective C++ doc pages?

Regards,
Gilles

>
> [1] https://en.cppreference.com/w/c/numeric/complex/cacos 
> 
> [2] 
> https://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#atan2-double-double-
>  
> 
>
> Alex
>

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [numbers] Complex missing some C++ standards

[Alex Herbert]

I’ve dropped the static equals methods and reciprocal and pushed the updated 
class with MathJax. 

I put MathJax in whenever possible. This may be a bit too much. The rendered 
javadoc looks good but the javadoc rendered by my IDE without MathJax support 
can be very unreadable.

Have a look at the built javadocs and through an IDE and let me know your 
opinions on the current usage.

It may be better to drop some use of MathJax such as \( z \) for {@code z} 
which would make the code more readable in an IDE when programming.

I’ve not explicitly laid out the latex equations for unparsed readability so 
some improvements could be made. However some equations are multi-line which 
gets wrapped to a single line if pure HTML without MathJax. For example see 
atanh and acos. I do not know how to lay this out to make it readable without 
MathJax. So perhaps a note in the class javadoc that the use of MathJax is 
required to read the formatted equations.

On another c++ note the documentation for the C99 functions on C++ reference 
lists the special return values, e.g. [1]. This is similar to the special 
return values listed in java.util.Math for functions, e.g. [2]. I think it 
would be good to add these to the javadoc. It should be a simple cut and 
reformat from the ISO C99 Annex G that the class has been tested against.

[1] https://en.cppreference.com/w/c/numeric/complex/cacos 

[2] 
https://docs.oracle.com/javase/8/docs/api/java/lang/Math.html#atan2-double-double-
 


Alex

Re: [numbers] Complex missing some C++ standards

[Gilles Sadowski]

Hi.

2019-12-29 1:15 UTC+01:00, Alex Herbert :
>
>
>> On 21 Dec 2019, at 11:42, Gilles Sadowski  wrote:
>>
>>> ...
>>
>> So, would you suggest that no "Number"-like class should ever throw
>> an exception (but instead return the equivalent of "Double.NaN”)?
>
> Yes. As it was the method could throw for some invalid input and not others.
> This is inconsistent. Changing to return a NaN equivalent is more neutral.
> Through the rest of the code all the computations that may end up computing
> NaN just return NaN. There is no raising of exceptions even when the ISO C99
> standard states that an exception may be raised, i.e. these do not throw
> ArithmeticException. Removing all exceptions from the class could be stated
> as a design decision in the class javadoc. The only exceptions are from the
> parse(String) method.

+1
Could also be considered as a simplification (no wondering about
which exception raise...).
Acceptable rationale for not throwing is that caller code can always
do the checks and act accordingly (throw or propagate NaN).

>
> I have been documenting the class using MathJax. This has raised a few more
> discrepancies with the c++ standards:
>
> 1. Static constructors
>
> Complex:
>
> ofCartesian(x, y)
> ofPolar(rho, theta)
>
> No public constructor.
>
> C++
>
> complex(x, y) // This is a public class constructor for type
> 
> polar(rho, theta)
>
> Should we change to the same name,

-0

> add synonyms

-0

> just accept the API
> difference?
>
> Sticking to the VALJO design would leave it as is and document that ofPolar
> matches the functionality of the ISO C++ polar method.

+1
Priority (IMO) would be to try and establish consistency within Java.

>
>
> 2. Reciprocal
>
> I had to change the implementation of reciprocal to call divide,
> effectively:
>
> Complex.ONE.divide(this)
>
> This uses scaling for the divisor and can compute values that the previous
> version could not such as:
>
> Complex.ofCartesian(Double.MAX_VALUE, Double.MAX_VALUE).reciprocal()
>
> It also better handles NaN and infinite edge cases.

Great.

>
> This method seems redundant. The origin in CM3 was due to Complex
> implementing the FieldElement interface. This has reciprocal() as a required
> method.
>
> It also has multiply(int) hence why that method was originally in the
> numbers version of Complex. This has now been dropped as it is redundant
> with multiply(double). Since we do not have to support the FieldElement
> interface I would suggest dropping reciprocal as well.

+1

>
> 3. Equals
>
> Why are there so many static equals functions using
> o.a.c.numbers.core.Precision with ULP and deltas?
>
> boolean equals(Complex x, Complex y, int maxUlps)
>
> @return {@code true} if there are fewer than {@code maxUlps} floating
>  * point values between the real or imaginary, respectively, parts of
> {@code x}
>  * and {@code y}.
>
>
> boolean equals(Complex x, Complex y)
>
> @return equals(x, y, 1)
>
>
> boolean equals(Complex x, Complex y, double eps)
>
> @return {@code true} if the values are two adjacent floating point
>  * numbers or they are within range of each other.
>
>
> boolean equalsWithRelativeTolerance(Complex x, Complex y,  double eps)
>
> {@code true} if the values are two adjacent floating point
>  * numbers or they are within range of each other.
>
>
> These are all helper methods. None are required for the test suite. I do not
> see the need to have them in the core Complex class. The methods do not
> cover all possible ways to measure equality, just some using the Precision
> class. I would drop these and leave it to a user to decide how to measure
> equality.

+1

>
> However I do note that similar methods are in the CM3 Complex class. Leaving
> them here would make porting to numbers easy. I think that before the 1.0
> release  is the time to decide if they should be maintained in Complex, in
> another class such as ComplexPrecision for legacy support of porting from
> CM3, or dropped. In the later case a note could be added to the package
> javadoc to state how to replicate the equals functionality from CM3 using
> numbers.Precision.

Let's drop, and wait for actual use cases.

Thanks,
Gilles

>
> Alex

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [numbers] Complex missing some C++ standards

[Alex Herbert]

> On 21 Dec 2019, at 11:42, Gilles Sadowski  wrote:
> 
>> ...
> 
> So, would you suggest that no "Number"-like class should ever throw
> an exception (but instead return the equivalent of "Double.NaN”)?

Yes. As it was the method could throw for some invalid input and not others. 
This is inconsistent. Changing to return a NaN equivalent is more neutral. 
Through the rest of the code all the computations that may end up computing NaN 
just return NaN. There is no raising of exceptions even when the ISO C99 
standard states that an exception may be raised, i.e. these do not throw 
ArithmeticException. Removing all exceptions from the class could be stated as 
a design decision in the class javadoc. The only exceptions are from the 
parse(String) method.


I have been documenting the class using MathJax. This has raised a few more 
discrepancies with the c++ standards:

1. Static constructors

Complex:

ofCartesian(x, y)
ofPolar(rho, theta)

No public constructor.

C++

complex(x, y) // This is a public class constructor for type 
polar(rho, theta)

Should we change to the same name, add synonyms or just accept the API 
difference?

Sticking to the VALJO design would leave it as is and document that ofPolar 
matches the functionality of the ISO C++ polar method.


2. Reciprocal

I had to change the implementation of reciprocal to call divide, effectively:

Complex.ONE.divide(this)

This uses scaling for the divisor and can compute values that the previous 
version could not such as:

Complex.ofCartesian(Double.MAX_VALUE, Double.MAX_VALUE).reciprocal()

It also better handles NaN and infinite edge cases.


This method seems redundant. The origin in CM3 was due to Complex implementing 
the FieldElement interface. This has reciprocal() as a required method. 

It also has multiply(int) hence why that method was originally in the numbers 
version of Complex. This has now been dropped as it is redundant with 
multiply(double). Since we do not have to support the FieldElement interface I 
would suggest dropping reciprocal as well.


3. Equals

Why are there so many static equals functions using 
o.a.c.numbers.core.Precision with ULP and deltas?

boolean equals(Complex x, Complex y, int maxUlps)

@return {@code true} if there are fewer than {@code maxUlps} floating
 * point values between the real or imaginary, respectively, parts of 
{@code x}
 * and {@code y}.


boolean equals(Complex x, Complex y)

@return equals(x, y, 1)


boolean equals(Complex x, Complex y, double eps)

@return {@code true} if the values are two adjacent floating point
 * numbers or they are within range of each other.


boolean equalsWithRelativeTolerance(Complex x, Complex y,  double eps)

{@code true} if the values are two adjacent floating point
 * numbers or they are within range of each other.


These are all helper methods. None are required for the test suite. I do not 
see the need to have them in the core Complex class. The methods do not cover 
all possible ways to measure equality, just some using the Precision class. I 
would drop these and leave it to a user to decide how to measure equality. 

However I do note that similar methods are in the CM3 Complex class. Leaving 
them here would make porting to numbers easy. I think that before the 1.0 
release  is the time to decide if they should be maintained in Complex, in 
another class such as ComplexPrecision for legacy support of porting from CM3, 
or dropped. In the later case a note could be added to the package javadoc to 
state how to replicate the equals functionality from CM3 using 
numbers.Precision.

Alex





-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [numbers] Complex missing some C++ standards

[Gilles Sadowski]

Hi.

2019-12-21 1:07 UTC+01:00, Alex Herbert :
>
>
>> On 20 Dec 2019, at 23:45, Gilles Sadowski  wrote:
>>
>> Le sam. 21 déc. 2019 à 00:05, Alex Herbert > > a écrit :
>>>
>>> Looking at the c++ standard [1] we are missing this function:
>>>
>>> norm() = a^2 + b^2
>>>
>>> The field norm of the complex, or the square of the absolute. An example
>>> on C++ reference states that this is faster for comparing magnitudes for
>>> ranking as it avoids the sqrt() required in abs().
>>>
>>> z.abs() > y.abs()  ==  z.norm() > y.norm()
>>>
>>>
>>> I suggest this is added to comply with the standard.
>>
>> +1
>>
>>>
>>>
>>> It seems odd to me that the constructor ofPolar throws an exception. It
>>> does this when the magnitude (rho) for the complex is negative. However
>>> if the magnitude is NaN it will not throw an exception and will end up
>>> returning NaN. I think this should be changed to return NaN for negative
>>> magnitude and avoid exceptions. This is basically stating that the polar
>>> representation you used is invalid and so in the Cartesian representation
>>> it will be (nan, nan).
>>>
>>> The C++ standard on this was previously vague and was clarified in [2]:
>>>
>>> “
>>> -?- Requires: rho shall be non-negative and non-NaN. theta shall be
>>> finite.
>>>
>>> -9- Returns: The complex value corresponding to a complex number whose
>>> magnitude is rho and whose phase angle is theta.
>>> “
>>>
>>> The assumption is that abs(polar(rho, theta)) == rho.
>>>
>>> If this cannot be ensured then polar(rho, theta) is undefined and we
>>> return NaN.
>>>
>>>
>>> Note that if theta is finite and rho is non-negative and non-nan:
>>>
>>> x = rho * Math.cos(theta)
>>> y = rho * Math.sin(theta)
>>>
>>> In the event that sin(theta) is zero (i.e. theta is zero) then inf * 0 is
>>> NaN. In this case the complex could either be:
>>>
>>> (Inf, nan) or (inf, 0)
>>>
>>> I have tried 2 c++ implementations and both return (inf, nan). The c++
>>>  header I have found would return (inf, 0). The same header also
>>> corrects if cos(theta) is zero however in Java cos(pi/2) is not zero so
>>> this is not an issue.
>>>
>>> Note:
>>> If the result is (inf, nan) then abs((inf, nan)) should return inf to
>>> satisfy the contract abs(polar(rho, theta)) == rho. This is currently
>>> true as abs() uses Math.hypot(x, y) which will return positive infinity
>>> if either argument is infinite. So I do not think it matters. An infinite
>>> is infinite even when the other part is nan.
>>>
>>>
>>> I suggest we update ofPolar to not throw exceptions and return NAN unless
>>> theta is finite and rho is non-negative and non-nan.
>>
>> +0
>> Not sure how useful it is to instantiate a useless object.
>
> It would not be instantiated. It can use the singleton NaN. But I do see
> your point. Something has gone wrong so raise it as a problem.
>
> Looking at various implementations the polar(rho, theta) method is used when
> computing results in polar coordinates to then return a result. For example
> this is used in various sqrt() implementations as the equivalent of:
>
> Complex.ofPolar(Math.sqrt(z.abs()), z.arg() / 2)
>
> In this case it would not be nice to throw an exception for a complex z
> which is NaN. Instead return NaN as the sqrt of NaN.
>
> I thought it would be more in line with the c++ standard to not throw
> exceptions. I also think that returning NaN is more in line with how
> java.util.Math handles invalid cases.

So, would you suggest that no "Number"-like class should ever throw
an exception (but instead return the equivalent of "Double.NaN")?

Perhaps that's the most neutral approach (careful users will check
the return value; others will have to use a debugger in order to find
the cause of the failure).
In the absence of other arguments, it's indeed safer to adopt the
same conventions as C++.

Regards,
Gilles

>>>
>>> Alex
>>>
>>>
>>> [1] http://www.cplusplus.com/reference/complex/
>>> 
>>> >> >
>>> [2] https://cplusplus.github.io/LWG/issue2459
>>> 
>>> >> >

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Re: [numbers] Complex missing some C++ standards

[Alex Herbert]

> On 20 Dec 2019, at 23:45, Gilles Sadowski  wrote:
> 
> Le sam. 21 déc. 2019 à 00:05, Alex Herbert  > a écrit :
>> 
>> Looking at the c++ standard [1] we are missing this function:
>> 
>> norm() = a^2 + b^2
>> 
>> The field norm of the complex, or the square of the absolute. An example on 
>> C++ reference states that this is faster for comparing magnitudes for 
>> ranking as it avoids the sqrt() required in abs().
>> 
>> z.abs() > y.abs()  ==  z.norm() > y.norm()
>> 
>> 
>> I suggest this is added to comply with the standard.
> 
> +1
> 
>> 
>> 
>> It seems odd to me that the constructor ofPolar throws an exception. It does 
>> this when the magnitude (rho) for the complex is negative. However if the 
>> magnitude is NaN it will not throw an exception and will end up returning 
>> NaN. I think this should be changed to return NaN for negative magnitude and 
>> avoid exceptions. This is basically stating that the polar representation 
>> you used is invalid and so in the Cartesian representation it will be (nan, 
>> nan).
>> 
>> The C++ standard on this was previously vague and was clarified in [2]:
>> 
>> “
>> -?- Requires: rho shall be non-negative and non-NaN. theta shall be finite.
>> 
>> -9- Returns: The complex value corresponding to a complex number whose 
>> magnitude is rho and whose phase angle is theta.
>> “
>> 
>> The assumption is that abs(polar(rho, theta)) == rho.
>> 
>> If this cannot be ensured then polar(rho, theta) is undefined and we return 
>> NaN.
>> 
>> 
>> Note that if theta is finite and rho is non-negative and non-nan:
>> 
>> x = rho * Math.cos(theta)
>> y = rho * Math.sin(theta)
>> 
>> In the event that sin(theta) is zero (i.e. theta is zero) then inf * 0 is 
>> NaN. In this case the complex could either be:
>> 
>> (Inf, nan) or (inf, 0)
>> 
>> I have tried 2 c++ implementations and both return (inf, nan). The c++ 
>>  header I have found would return (inf, 0). The same header also 
>> corrects if cos(theta) is zero however in Java cos(pi/2) is not zero so this 
>> is not an issue.
>> 
>> Note:
>> If the result is (inf, nan) then abs((inf, nan)) should return inf to 
>> satisfy the contract abs(polar(rho, theta)) == rho. This is currently true 
>> as abs() uses Math.hypot(x, y) which will return positive infinity if either 
>> argument is infinite. So I do not think it matters. An infinite is infinite 
>> even when the other part is nan.
>> 
>> 
>> I suggest we update ofPolar to not throw exceptions and return NAN unless 
>> theta is finite and rho is non-negative and non-nan.
> 
> +0
> Not sure how useful it is to instantiate a useless object.

It would not be instantiated. It can use the singleton NaN. But I do see your 
point. Something has gone wrong so raise it as a problem.

Looking at various implementations the polar(rho, theta) method is used when 
computing results in polar coordinates to then return a result. For example 
this is used in various sqrt() implementations as the equivalent of:

Complex.ofPolar(Math.sqrt(z.abs()), z.arg() / 2)

In this case it would not be nice to throw an exception for a complex z which 
is NaN. Instead return NaN as the sqrt of NaN.

I thought it would be more in line with the c++ standard to not throw 
exceptions. I also think that returning NaN is more in line with how 
java.util.Math handles invalid cases.


> 
> Regards,
> Gilles
> 
>> 
>> Alex
>> 
>> 
>> [1] http://www.cplusplus.com/reference/complex/ 
>>  
>> > >
>> [2] https://cplusplus.github.io/LWG/issue2459 
>>  
>> > >
>> 
>> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org 
> 
> For additional commands, e-mail: dev-h...@commons.apache.org 
> 

Re: [numbers] Complex missing some C++ standards

[Gilles Sadowski]

Le sam. 21 déc. 2019 à 00:05, Alex Herbert  a écrit :
>
> Looking at the c++ standard [1] we are missing this function:
>
> norm() = a^2 + b^2
>
> The field norm of the complex, or the square of the absolute. An example on 
> C++ reference states that this is faster for comparing magnitudes for ranking 
> as it avoids the sqrt() required in abs().
>
> z.abs() > y.abs()  ==  z.norm() > y.norm()
>
>
> I suggest this is added to comply with the standard.

+1

>
>
> It seems odd to me that the constructor ofPolar throws an exception. It does 
> this when the magnitude (rho) for the complex is negative. However if the 
> magnitude is NaN it will not throw an exception and will end up returning 
> NaN. I think this should be changed to return NaN for negative magnitude and 
> avoid exceptions. This is basically stating that the polar representation you 
> used is invalid and so in the Cartesian representation it will be (nan, nan).
>
> The C++ standard on this was previously vague and was clarified in [2]:
>
> “
> -?- Requires: rho shall be non-negative and non-NaN. theta shall be finite.
>
> -9- Returns: The complex value corresponding to a complex number whose 
> magnitude is rho and whose phase angle is theta.
> “
>
> The assumption is that abs(polar(rho, theta)) == rho.
>
> If this cannot be ensured then polar(rho, theta) is undefined and we return 
> NaN.
>
>
> Note that if theta is finite and rho is non-negative and non-nan:
>
> x = rho * Math.cos(theta)
> y = rho * Math.sin(theta)
>
> In the event that sin(theta) is zero (i.e. theta is zero) then inf * 0 is 
> NaN. In this case the complex could either be:
>
> (Inf, nan) or (inf, 0)
>
> I have tried 2 c++ implementations and both return (inf, nan). The c++ 
>  header I have found would return (inf, 0). The same header also 
> corrects if cos(theta) is zero however in Java cos(pi/2) is not zero so this 
> is not an issue.
>
> Note:
> If the result is (inf, nan) then abs((inf, nan)) should return inf to satisfy 
> the contract abs(polar(rho, theta)) == rho. This is currently true as abs() 
> uses Math.hypot(x, y) which will return positive infinity if either argument 
> is infinite. So I do not think it matters. An infinite is infinite even when 
> the other part is nan.
>
>
> I suggest we update ofPolar to not throw exceptions and return NAN unless 
> theta is finite and rho is non-negative and non-nan.

+0
Not sure how useful it is to instantiate a useless object.

Regards,
Gilles

>
> Alex
>
>
> [1] http://www.cplusplus.com/reference/complex/ 
> 
> [2] https://cplusplus.github.io/LWG/issue2459 
> 
>
>

-
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org