Page 2 of 4
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Fri Mar 24, 2017 10:11 am
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.
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Fri Mar 24, 2017 10:16 am
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]
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Fri Mar 24, 2017 4:05 pm
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.
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Fri Mar 24, 2017 5:46 pm
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.
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Sat Mar 25, 2017 4:06 am
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.
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Sat Mar 25, 2017 8:45 am
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.
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Mon Mar 27, 2017 8:10 am
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
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Mon Mar 27, 2017 9:24 am
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
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Tue Mar 28, 2017 8:10 pm
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.
Re: Issue-#157 Extending the domain of Math.Sin, Cos, & Tan
Posted: Tue Mar 28, 2017 8:14 pm
by Robert
Robert wrote:Some wording for discussion:
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)