tc39 / test262

Official ECMAScript Conformance Test Suite
Other
2.41k stars 468 forks source link

Math.sqrt: add a test with exact input-output expectations #4188

Closed michaelficarra closed 3 months ago

michaelficarra commented 3 months ago

Tests for this ecma262 PR: https://github.com/tc39/ecma262/pull/3345

We could additionally test chosen interesting/"edge-case" inputs, but I wouldn't know how to discover those. @anba @ptomato suggested that you may have some recommendations for these or for inputs that might have different results using known sqrt approximation strategies.

Input-output pairs were generated by the following script:

let taInt32 = new Uint32Array(2);
let taFloat64 = new Float64Array(taInt32.buffer);
function intsToFloat(low, high) {
    taInt32[0] = low;
    taInt32[1] = high;
    return taFloat64[0];
}
function floatToInts(f) {
  taFloat64[0] = f;
  return [taInt32[0], taInt32[1]];
}

const DOUBLE_BIAS = 1024;

let results = new Map;
for (let tests = 0; tests < 1000; ++tests) {
  let [low, high] = crypto.getRandomValues(new Uint32Array(2));
  high &= 0b0_00000000000_11111111111111111111;
  high |= (((Math.random() < 0.5 ? -1 : 1) * tests + DOUBLE_BIAS) << 20);
  let candidate = intsToFloat(low, high);
  results.set(candidate, Math.sqrt(candidate));
}

console.log(JSON.stringify([...results.entries()]));
michaelficarra commented 3 months ago

I'm not sure why esmeta is getting a stack overflow.

ioannad commented 3 months ago

I'm not sure why esmeta is getting a stack overflow.

@michaelficarra FWIW I just tried running the test in my local clone of esmeta pointing to the same HEAD as in the github checks here and the test passed. I restarted the test to see if it fails again.

michaelficarra commented 3 months ago

@ioannad That seems to have worked. It no longer gets a stack overflow, but it does fail. It opaquely fails, so I'm not sure which assertion caused the failure. Also, I'm pretty sure the test should pass. I ran the test manually in V8 and it passed there.

michaelficarra commented 3 months ago

@Ms2ger Spec PR is merged. esmeta is getting a stack overflow again after rebase. 🤷‍♂️

Ms2ger commented 3 months ago

@doehyunbaek @Zaid-maker either of you able to shed some light on esmeta's stack overflow? Otherwise I'll probably merge this anyway in the next few days

jhnaldo commented 3 months ago

Hi all, I'm the maintainer of ESMeta. It happens because of the limited stack size of the released executable (bin/esmeta). So, I added the option -Xss4m for released executables in https://github.com/es-meta/esmeta/pull/254. I'm sorry about this issue. I believe the latest version of ESMeta v0.4.2 will fix this problem. If it does not work, please let me know.

jhnaldo commented 3 months ago

In addition, we checked that the fixed version passed the test as follows:

$ esmeta test262-test -test262-test:log results.js
========================================
 extract phase
----------------------------------------
========================================
 compile phase
----------------------------------------
========================================
 build-cfg phase
----------------------------------------
========================================
 test262-test phase
----------------------------------------
[Interpreter] Logging into /Users/naldo/project/esmeta/logs/test262/log ...
[Interpreter] Logging finished
PASS
Ms2ger commented 3 months ago

Thanks for the swift response! I've opened #4202 to update our CI, and then we can go ahead here when that is landed.

michaelficarra commented 3 months ago

@Ms2ger esmeta test passes now. Should be good to go.

anba commented 3 months ago

@anba @ptomato suggested that you may have some recommendations for these or for inputs that might have different results using known sqrt approximation strategies.

I've checked Bugzilla for sqrt precision bugs, but I didn't find any. Probably because the implementation uses std:sqrt, which is known to be correct. Comparing the inputs against naive implementations (*) shows some mismatches, so I guess the randomly generated inputs are good enough.

(*) For example a naive implementation of Goldschmidt's algorithm in JS shows ~380 mismatches, with a maximum error of 2 ULP.