[BUG]: C implementation for `stats/base/dists/triangular/mgf` has different results than the JavaScript implementation

[kgryte]

Description

For context, see the commit message for c88e9f5.

In short, when testing the C implementation, we need to increase test tolerances by many orders of magnitude in order to get tests to pass. It would be great to figure out why and track down this bug. In my investigations, I was not able to ascertain the cause of the divergence.

Related Issues

None.

Questions

No.

Demo

No response

Reproduction

- Reduce the test tolerances for the native tests to the same as the JavaScript tests.
- Run `make install-node-addons NODE_ADDONS_PATTERN="stats/base/dists/triangular/mgf"`
- Run `make test TESTS_FILTER=".*/stats/base/dists/triangular/mgf/.*"`

Expected Results

I would expect that the JavaScript and C results would be approximately the same.

Actual Results

Version

No response

Environments

N/A

Browser Version

No response

Node.js / npm Version

No response

Platform

No response

Checklist

• Read and understood the Code of Conduct.
• Searched for existing issues and pull requests.

[vivekmaurya001]

Hi @kgryte , i tested the native file and found that their is no failure of test case on reducing the tolrance as all the calculated mgf values are exactly equal to expected values

[kgryte]

After setting the tolerances to the same as the JavaScript test file (i.e., 1.0 * EPS), compiling the add-on, and then running the tests, I see over 500 values which fail the tolerance tests. Note, however, that the number of failures could be compiler-specific, if the cause of the divergence is due to particular compiler optimizations (e.g., term reordering, operator fusion, etc).

[Vinit-Pandit]

In my setup all the test cases are passing after setting the tolerance in test.native.js to 1.0 * EPS.

[anandkaranubc]

In my setup all the test cases are passing after setting the tolerance in test.native.js to 1.0 * EPS.

Interesting. Did you change the tol in all the three places (small, medium, large ranges)?

[kgryte]

@anandkaranubc I did, but I was not scientific in my approach.

[kgryte]

As discussed during office hours, I think it best if we first updated our MGF implementation to match the most recent implementation in Julia stats. May require updating test fixtures, etc.

[anandkaranubc]

As discussed during office hours, I think it best if we first updated our MGF implementation to match the most recent implementation in Julia stats. May require updating test fixtures, etc.

Will raise a PR shortly. Thanks for the help, Athan!