Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Merged to the master branch
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Ivan Denisov »

Robert wrote:Regarding the Docu pre-condition I still think that there should be no number. Neither 20 or 143 is visible to the non system programmer who uses the Math.Sin function.
Why it is invisible? This message will be shown in the TRAP window. So there should be meaningful number.
User avatar
Robert
Posts: 1024
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Robert »

Ivan Denisov wrote:
Robert wrote:Regarding the Docu pre-condition I still think that there should be no number. Neither 20 or 143 is visible to the non system programmer who uses the Math.Sin function.
Why it is invisible? This message will be shown in the TRAP window. So there should be meaningful number.
Test code & TRAP extract - no number visible.

Code: Select all

    z  :=  Math.Sin (INF);
...
undefined real result  (F8A1, 37E)
Pre: ABS(x) # INF

 Math.Sin   [000002E5H] 
	.x	REAL	inf
 RdcTest.DoSin   [00000732H] 
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Ivan Denisov »

Oh, I understood now how it works. Is something traps inside the function, there is possibility to make informative message! Brilliant system!

Code: Select all

Math.Sqrt.143	Pre: argument of Sqrt must not be negative
Math.Ln.143	Pre: argument of Ln must not be negative
Math.Log.143	Pre: argument of Log must not be negative
Math.Sin.143	Pre: ABS(x) # INF
Math.Cos.143	Pre: ABS(x) # INF
Math.Tan.143	Pre: ABS(x) # INF
Math.ArcSin.143	Pre: -1.0 <= x <= 1.0
Math.ArcCos.143	Pre: -1.0 <= x <= 1.0
I think, that is better not change Ominc tradition in this case. They have 20. Let's keep 20.
User avatar
Robert
Posts: 1024
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Robert »

If we changed the Strings file (which we could) to read

Code: Select all

Math.Sin.143	Math.Sin Precondition #20: ABS(x) # INF
or something similar it would make sense for the code comment to say
"(* 20, ABS(x) # INF *)" and for the Docu to refer to Precondition #20.
But, at the moment, the 20 has no significance and is better removed.
Brilliant system!
Yes, but it is not brilliantly explained in any Docu I know about.
Ivan Denisov
Posts: 1700
Joined: Tue Sep 17, 2013 12:21 am
Location: Russia

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Ivan Denisov »

Robert, I think that 20 here is the short way to say "precondition". If you will remove it, the part of semantics will be lost. And 143 is not good here, because it will change the meaning of the comment. So I prefer you keep it like Oberon microsystems did.
User avatar
Robert
Posts: 1024
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Robert »

Ivan Denisov wrote:Robert, I think that 20 here is the short way to say "precondition".
Ok.
I prefer no number in the comment & Docu.
JT prefers 143.
ID prefers 20.

I don't feel very strongly, and hope this discussion is not too long.
I can agree to 20 in both places - ie no change.
I can agree to 143 in the comment, if the comment is clarified to say it refers to the kernel error. I do not like 143 in the Docu.
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Josef Templ »

Robert wrote: The manual says:
Description
Computes both the sine and the cosine of the source operand in register ST(0),
stores the sine in ST(0), and pushes the cosine onto the top of the FPU register stack.
(This instruction is faster than executing the FSIN and FCOS instructions in succes-
sion.)
which might mean it only stacks one value, so only one pop is required?
In addition, the manual says that in case of a range error ST(0) is unchanged.
This means that there is no second result pushed onto the stack in case of a range overflow
and the implementation is correct.

- Josef
User avatar
Josef Templ
Posts: 2047
Joined: Tue Sep 17, 2013 6:50 am

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Josef Templ »

Robert wrote:
Ivan Denisov wrote:Robert, I think that 20 here is the short way to say "precondition".
Ok.
I prefer no number in the comment & Docu.
JT prefers 143.
ID prefers 20.

I don't feel very strongly, and hope this discussion is not too long.
I can agree to 20 in both places - ie no change.
I can agree to 143 in the comment, if the comment is clarified to say it refers to the kernel error. I do not like 143 in the Docu.
Note: So far I was only talking about the error number in the source comment, not in the docu.
The number in the source comment gives you a chance to see to which Strings key it refers to.
Otherwise you have to study the Kernel's trap handler in full detail to see the link.
With respect to the source comment this case is not a matter of personal preferences, I think.
20 is wrong, 143 is correct. It can of course be labelled as "Kernel error number" and it may also be
a good idea to add in a comment that it is checked internally by FSIN, FSINCOS, etc. because
this is also not obvious from the source code.

I agree that the error number in the docu is misleading either way.
20 is wrong because it is not the number being used. 143 is an internally used number
outside the valid range of 0 .. 128.
It should better be removed from the docu.

- Josef
User avatar
Robert
Posts: 1024
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Robert »

Robert wrote:Mystery on mystery!
The MATLAB results are all correct to the 7 digits quoted. It must be using multi-precision of some kind, which must be slow?
I've done some timings:
For small numbers (x = 5.) MATLAB and BlackBox are similar in speed. For big (x = several million) and very big (x = 5.E18) MATLAB slows down by a factor of 10; BlackBox, using the native FPU, maintains its speed.

There is quite a bit of discussion on the web about using elaborate software to improve the accuracy of the Intel FPU FSIN instruction. Personally I don't think we should consider doing that for the standard BlackBox library; the Intel hardware strikes me as a very reasonable compromise for general purpose use.

I've also timed the SinCos procedure. It is only very slightly slower than calling one of Sin or Cos. This makes it a very desirable addition to the library.
Last edited by Robert on Tue Mar 28, 2017 8:16 pm, edited 1 time in total.
User avatar
Robert
Posts: 1024
Joined: Sat Sep 28, 2013 11:04 am
Location: Edinburgh, Scotland

Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan

Post by Robert »

Robert wrote:Some wording for discussion:
ArcCot (x) = Math.Pi() / 2.0 - Math.ArcTan(x)
(* Refs: https://en.wikipedia.org/wiki/Inverse_t ... _functions, Maple *)

ArcCot (x) = Math.ArcTan(1.0 / x)
(* Refs: http://dlmf.nist.gov/search/search?q=arccot, MATLAB, Mathematica, Maxima, MuPAD, Wikipedia *)
No comments!
I'm going to make a simpler proposal: Remove
ArcCot (x) = Math.Pi() / 2.0 - Math.ArcTan(x)
and insert
ArcCot (x) = Math.ArcTan(1.0 / x)
Post Reply