Feature #1597: shift SQRT implementation (MathOps.sqrt()) to BigDecimal - Base Language - Golden Code Redmine

Feature #1597

Feature #1596: complete big decimal implementation

shift SQRT implementation (MathOps.sqrt()) to BigDecimal

Added by Greg Shah over 11 years ago. Updated over 7 years ago.

Status:

Closed

Priority:

Normal

Assignee:

Hynek Cihlar

Target version:

FWD - Cleanup and Stablization for Server Features

Start date:

01/24/2014

Due date:

01/28/2014

% Done:

100%

Estimated time:

6.00 h

billable:

vendor_id:

GCD

hc_upd20140202a.zip (19.9 KB) Hynek Cihlar, 02/02/2014 06:42 PM

hc_upd20140203a.zip (9.98 KB) Hynek Cihlar, 02/03/2014 05:06 AM

hc_upd20140203b.zip (9.96 KB) Hynek Cihlar, 02/03/2014 09:51 AM

hc_upd20140205a.zip (10.4 KB) Hynek Cihlar, 02/05/2014 07:20 PM

hc_upd20140206a.zip (10.3 KB) Hynek Cihlar, 02/06/2014 10:02 AM

hc_upd20140209a.zip (68.2 KB) Hynek Cihlar, 02/12/2014 10:23 AM

History

#1 Updated by Greg Shah over 11 years ago

Target version set to Milestone 7

#2 Updated by Greg Shah almost 11 years ago

Target version changed from Milestone 7 to Milestone 11

#3 Updated by Greg Shah almost 11 years ago

Assignee set to Ovidiu Maxiniuc
Start date changed from 10/19/2012 to 08/06/2013
Due date set to 08/06/2013

#4 Updated by Greg Shah about 10 years ago

Due date changed from 08/06/2013 to 01/28/2014
Start date changed from 08/06/2013 to 01/24/2014
Assignee changed from Ovidiu Maxiniuc to Hynek Cihlar

#5 Updated by Hynek Cihlar about 10 years ago

What is the expected precision of the algorithm?

#6 Updated by Greg Shah about 10 years ago

I don't know the answer. Often, if the input is a decimal value, then the output can have the same precision. Otherwise, usually the precision is the decimal default (10 digits).

Please write some test code to explore this and to prove that the result is compatible with the 4GL for a representative range of input values.

#7 Updated by Hynek Cihlar about 10 years ago

Status changed from New to WIP

#8 Updated by Hynek Cihlar about 10 years ago

File hc_upd20140202a.zip added

Attached are the changes for initial review.

The implemented algorithm yields the same results as the native Progress code for numbers less than 99999999999999 or so. For greater numbers, the implemented algorithm calculates results with smaller errors.

I attempted to modify the conversion to emit the sqrt call with a decimal argument (ex. sqrt(new decimal("1"))) but didn't succeed. See SignatureHelper.java in the attached file.

#9 Updated by Hynek Cihlar about 10 years ago

File hc_upd20140203a.zip added

I didn't realize the evaluator already casts the operands to proper types. Attached is the updated code.

#10 Updated by Hynek Cihlar about 10 years ago

File hc_upd20140203b.zip added

Attached file changes the precision of the sqrt algorithm to be defined by decimal.MAX_SCALE.

#11 Updated by Hynek Cihlar about 10 years ago

Status changed from WIP to Review

#12 Updated by Greg Shah about 10 years ago

Code Review 0203b

Overall, this is very good.

1. Should we put a quick out for BigDecimal.ONE?

2. Add javadoc to describe the 4GL behavior of SQRT and how this algorithm matches (and more importantly, deviates). For example, for negative numbers the 4GL will return unknown value. This algorithm does the same (indirectly) by returning null, which will yield a decimal instance that is unknown.

3. Please check in your 4GL testcases.

#13 Updated by Hynek Cihlar about 10 years ago

File hc_upd20140205a.zip added

The input of one is covered by the initial approximation, but yes it won't hurt if stated explicitly.

New in the attached file:

quick out for one
more docs including comparison with native Progress
quick out for unknown value because sqrt(?) == 1

Also the testcases file is checked in. See testcases/uast/math/sqrt.p.

#14 Updated by Greg Shah about 10 years ago

Code Review 0205a

This looks good.

My only question is about this table:

    * Progress   Error: 0.0000209975   0.0010625323   0.2415973135   28.5870600263   6385341029806439311343616
    * 4GL        Error: 0.0000422480   0.0004300767   0.0026421128   0.00006997839   17801490614.8294175592

Should one of these lines show "P2J" instead of "Progress" or "4GL"? For us, "Progress" and "4GL" are the same thing and neither of them can be used to describe our P2J technology.