Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new tests for Math built-ins #269

Merged
merged 1 commit into from
May 16, 2015
Merged

Conversation

suwc
Copy link

@suwc suwc commented May 14, 2015

Add new tests for Math built-ins

@bterlson
Copy link
Member

LGTM. I'll wait and see if anyone else has comments before I merge :)

@bterlson
Copy link
Member

It's interesting to consider accuracy tests for these built-ins but since the spec doesn't mandate any accuracy guarantees I wonder if we should do them? We have the tests so we can contribute them... What do you guys think?

@anba
Copy link
Contributor

anba commented May 15, 2015

We have the tests so we can contribute them... What do you guys think?

Accuracy tests are already present for other Math built-ins (15.8.2.2_A5, 15.8.2.3_A6, 15.8.2.4_A6, ...), so I think it's ok to include them. But it may depend on the accepted error.

@bterlson
Copy link
Member

@suwc want to paste one of the accuracy tests in a comment and we can discuss?

IIRC the tests do not involve any epsilon value, so there is no accepted error. However, I suspect that the tests will pass on any implementation assuming it's not self-hosted in JS (as, I think, some implementations are now, or at least were). Further, I actually think such tests are valuable because it ensures implementations are actually providing the value we wanted them to provide by adding these methods to the spec... but pedantically, spec doesn't require it, so we have to be careful testing it :)

@suwc
Copy link
Author

suwc commented May 15, 2015

Here is an example:

// Copyright 2015 Microsoft Corporation. All rights reserved.
// This code is governed by the license found in the LICENSE file.

/*---
description: Accuracy tests for log1p
includes: [runTestCase.js]
es6id: 20.2.2.21
---*/

// [input, expectedOutput]
var data = [
[-0.693303108215332, -1.1818953422964992],
[-0.6500036120414734, -1.049832444670426],
[-0.5634434223175049, -0.8288372954181591],
[-0.546193540096283, -0.7900844716039174],
[-0.5425496697425842, -0.7820869679038186],
[-0.5222184658050537, -0.7386016923991179],
[-0.5103321671485901, -0.7140280098902131],
[-0.5013591051101685, -0.695869091822011],
[-0.47934168577194214, -0.6526612791947967],
[-0.436265230178833, -0.573171404368468],
[-0.40330591797828674, -0.5163507223485767],
[-0.3905523121356964, -0.49520216169452136],
[-0.3101024627685547, -0.37121218918358967],
[-0.2841587960720062, -0.334296918842278],
[-0.2685661315917969, -0.31274846806577955],
[-0.19418840110301971, -0.21590531206566665],
[-0.14176605641841888, -0.15287855514257634],
[-0.10953421145677566, -0.11601059524620481],
[-0.02047410048544407, -0.02068660038044095],
[1.6900931765206905e-9, 1.690093175092483e-9],
[2.114990849122478e-9, 2.114990846885885e-9],
[7.09962844069878e-9, 7.099628415496418e-9],
[1.3671879628418537e-8, 1.3671879534958392e-8],
[1.6793412882520897e-8, 1.679341274151154e-8],
[5.86768322818898e-8, 5.8676830560404544e-8],
[1.1404608812881633e-7, 1.1404608162556172e-7],
[1.4555860161635792e-7, 1.4555859102270568e-7],
[3.891847768500156e-7, 3.8918470111764e-7],
[6.237826255528489e-7, 6.237824310005479e-7],
[0.0000010466960702615324, 0.0000010466955224755828],
[0.000002951089072666946, 0.0000029510847182121553],
[0.000004877083483734168, 0.000004877071590801183],
[0.000009066634447663091, 0.000009066593345981423],
[0.00002360353755648248, 0.0000236032589973732],
[0.00006081791070755571, 0.000060816061373405674],
[0.0001194767392007634, 0.0001194696024236053],
[0.00024370866594836116, 0.00024367897381548741],
[0.0004797091241925955, 0.00047959410055446765],
[0.0009607885731384158, 0.0009603273112237537],
[0.0011304814834147691, 0.0011298429703953875],
[0.003370779100805521, 0.003365110759179022],
[0.0051277875900268555, 0.0051146852588027005],
[0.007697627879679203, 0.007668152306886386],
[0.015477418899536132, 0.015358865355358764],
[0.030580732971429825, 0.03012246177452264],
[0.03468317911028862, 0.034095272717161014],
[0.0656340941786766, 0.06357001557994644],
[0.0661088228225708, 0.06401540573272318],
[0.09283597767353058, 0.0887761317609137],
[0.1853029727935791, 0.1699984151517874],
[0.19566869735717773, 0.1787056082557074],
[0.218036949634552, 0.19724050514493468],
[0.22476322948932648, 0.20274754326577834],
[0.2502291798591614, 0.22332687839610243],
[0.253903329372406, 0.22626134942448822],
[0.31617453694343567, 0.27472945096567547],
[0.40909022092819214, 0.3429442627531701],
[0.41710007190704345, 0.34861258059107824],
[0.417348176240921, 0.34878764417509206],
[0.42039263248443603, 0.35093333514320846],
[0.44061312079429626, 0.36506880129940994],
[0.4500701129436493, 0.37161190901771884],
[0.46901199221611023, 0.38459006068197926],
[0.48878103494644165, 0.39795768778172313],
[0.5298029184341431, 0.42513891562648054],
[0.5681086778640747, 0.44987022938869114],
[0.5787261724472046, 0.4566183018986475],
[0.5820297598838806, 0.45870868072597365],
[0.6075904965400696, 0.4747364719929956],
[0.6400337219238281, 0.4947168037733847],
[0.6405095458030701, 0.49500689223968214],
[0.6432893872261047, 0.4966999569758337],
[0.6485147476196289, 0.4998747295737267],
[0.6508439183235168, 0.5012866228091318],
[0.6547728776931763, 0.5036637653893262],
[0.6564148664474487, 0.5046555478044253],
[0.658829927444458, 0.506112490850477],
[0.6735535264015198, 0.5149492258613715],
[0.6900337934494018, 0.5247485248589687],
[0.6950458288192749, 0.5277097781183925],
];

function testcase() {
    for (var i in data) {
        if(Math.log1p(data[i][0]) !== data[i][1])
        {
            $ERROR("Math.log1p produces incorrect result for: " + data[i][0]);
        }
    }
    return true;
}
runTestCase(testcase);

@anba
Copy link
Contributor

anba commented May 15, 2015

Running the example in JavaScriptCore, SpiderMonkey, V8, and with Java's Math.log1p method:

Math.log1p produces incorrect result for: -0.693303108215332
Math.log1p produces incorrect result for: -0.2685661315917969
Math.log1p produces incorrect result for: 0.417348176240921

@bterlson
Copy link
Member

@anba those are the only three failures? What values do they produce (how close are they)? Maybe we can introduce an epsilon and these tests would be ok?

@bterlson
Copy link
Member

I added an agenda item to discuss this topic at the next meeting. In the meantime I'll pull this in by EOD since there doesn't seem to be feedback on this topic...

@bterlson bterlson closed this May 15, 2015
@bterlson bterlson reopened this May 15, 2015
@anba
Copy link
Contributor

anba commented May 16, 2015

@anba those are the only three failures? What values do they produce (how close are they)?

Yes, only these three tests fail.

// Changed test code:
for (var sample of data) {
  var input = sample[0];
  var expected = sample[1];
  var actual = Math.log1p(input);
  if (actual !== expected) {
    print(`Math.log1p(${input}) = ${actual}, but expected: ${expected}`);
  }
}

Formatted output and results from Wolfram|Alpha for comparison:

Math.log1p(-0.693303108215332) = -1.1818953422964995, but expected: -1.1818953422964993
Actual          = -1.1818953422964995
Expected        = -1.1818953422964993
Wolfram|Alpha   = -1.18189534229649927841059195430227691494585231491778309111349...
toString()      = -1.1818953422964993

Math.log1p(-0.2685661315917969) = -0.3127484680657796, but expected: -0.31274846806577955
Actual          = -0.3127484680657796
Expected        = -0.31274846806577955
Wolfram|Alpha   = -0.31274846806577960975373775206731342551299676699836258973612...
toString()      = -0.3127484680657796

Math.log1p(0.417348176240921) = 0.348787644175092, but expected: 0.34878764417509206
Actual          = 0.348787644175092
Expected        = 0.34878764417509206
Wolfram|Alpha   = 0.348787644175092020530375503464887895832767462276661113157550...
toString()      = 0.348787644175092

@bterlson
Copy link
Member

huh... well, the good news I guess is we can introduce a really small epsilon value to make this pass. Though I think we will have to discuss adding these tests after I get buyoff (or not) from tc39.

I'll merge this in as-is for now!

bterlson added a commit that referenced this pull request May 16, 2015
Add new tests for Math built-ins
@bterlson bterlson merged commit b56af07 into tc39:master May 16, 2015
@anba
Copy link
Contributor

anba commented May 16, 2015

huh... well, the good news I guess is we can introduce a really small epsilon value to make this pass.

The computed results are within 1 ulp of the exact results. TC39 should probably decide how large the acceptable error bound can be (per function).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants