cumulativeStdNormalProbability uses toFixed(), which returns a string. This is not ho

It's a normal distribution table, which has <a href="https://www.investopedia.com/arti

why is a math lib returning strings? about simple-statistics HOT 7 CLOSED

TheDirigible commented on June 14, 2024

why is a math lib returning strings?

from simple-statistics.

Comments (7)

mourner commented on June 14, 2024 1

I agree that the original form of the issue report was not very productive, but the underlying criticism is sound — toFixed should be used only for formatting, not for numeric operations. In this case it's better not only as a potential micro-optimization, but also for consistency, since the table values are rounded numerically (with Math.round(1e4 * value) / 1e4):

simple-statistics/src/standard_normal_table.js

Lines 12 to 15 in b4a9c78

 return ( 

 Math.round((0.5 + (sum / SQRT_2PI) * Math.exp((-z * z) / 2)) * 1e4) / 

 1e4 

 );

from simple-statistics.

tmcw commented on June 14, 2024

So, the lines are:

simple-statistics/src/cumulative_std_normal_probability.js

Lines 29 to 39 in b4a9c78

 // The index we calculate must be in the table as a positive value, 

 // but we still pay attention to whether the input is positive 

 // or negative, and flip the output value as a last step. 

 if (z >= 0) { 

 return standardNormalTable[index]; 

 } else { 

 // due to floating-point arithmetic, values in the table with 

 // 4 significant figures can nevertheless end up as repeating 

 // fractions when they're computed here. 

 return +(1 - standardNormalTable[index]).toFixed(4); 

 }

There's an explanatory comment explaining why, in this method that's trying to return things with 4 significant figures, it uses .toFixed.

And, the expression +(1 - standardNormalTable[index]).toFixed(4); does not return a string. Does it for you, or was that an assumption?

from simple-statistics.

TheDirigible commented on June 14, 2024

Ahh I see, the + returns it to a number. That doesn't improve things though, converting to string and back is a costly conversion. Not sure why you don't just return the value?

…

On Tue, Jul 11, 2023 at 5:27 AM Tom MacWright ***@***.***> wrote: So, the lines are: https://github.com/simple-statistics/simple-statistics/blob/b4a9c787943534d46ce8405846858eb4866af47e/src/cumulative_std_normal_probability.js#L29-L39 There's an explanatory comment explaining why, in this method that's trying to return things with 4 significant figures, it uses .toFixed. And, the expression +(1 - standardNormalTable[index]).toFixed(4); does not return a string. Does it for you, or was that an assumption? — Reply to this email directly, view it on GitHub <#716 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APDPBBM2T2HSZRQ2L43JQ7LXPVBBPANCNFSM6AAAAAA2FKLK2Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

from simple-statistics.

tmcw commented on June 14, 2024

It's a normal distribution table, which has either four or five digits of precision, in this case, four.

Converting to a string and back is not a costly conversion, relative to virtually everything else. If that's actually a bottleneck in your application, that'd be surprising, but if it is based on a profile, and you have a faster technique, I'm more than happy to merge a pull request to limit significant digits in a faster way.

from simple-statistics.

TheDirigible commented on June 14, 2024

Sure the value comes from a table with limited precision (I would argue too limited - 6 digits is better for finance), but there's no reason for this function to slow itself down in order to output the same. The point of a table lookup is to be fast. The consumer is looking for a fast result, not a value ready-made to be placed in a table. Also a linear interpolation between the 2 closest table entries would be more accurate. toFixed() is 20x slower than alternatives https://stackoverflow.com/questions/36529495/performance-tofixed-vs-math-floorx-10000

…

On Tue, Jul 11, 2023 at 6:07 AM Tom MacWright ***@***.***> wrote: It's a normal distribution table, which has either four or five digits of precision <https://www.investopedia.com/articles/active-trading/092914/normal-distribution-table-explained.asp>, in this case, four. Converting to a string and back is not a costly conversion, relative to virtually everything else. If that's actually a bottleneck in your application, that'd be surprising, but if it is based on a profile, and you have a faster technique, I'm more than happy to merge a pull request to limit significant digits in a faster way. — Reply to this email directly, view it on GitHub <#716 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APDPBBNMRKPKGCL7N6A5O33XPVF2XANCNFSM6AAAAAA2FKLK2Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

from simple-statistics.

tmcw commented on June 14, 2024

Okay, I'm happy to accept a PR if you want to write one, to improve performance here. I will say - I'm specifically talking about identifying toFixed as a bottleneck in a real-world program. Microbenchmarks can identify differences between Math.round versus the ~~ bit-twiddling hack, but 98% of those who care about that, including me in a previous iteration, are wrong to focus on it - the difference never rises to 1% of total-application performance budget, and usually gets optimized away once some V8 engineer decides to make Math.random or toFixed or etc faster.

I will say, I'm really happy to discuss this stuff, but in the future - this discussion started with an incorrect assumption based on a misreading of the code, led to a question answered by a comment in the code, and now is about microbenchmarking. It would be certainly nicer and more productive to have fewer assumptions and a generally more collaborative intent.

from simple-statistics.

TheDirigible commented on June 14, 2024

I'm helping by pointing out the problem, sorry I don't have time to fix it. I don't even use this lib. One of my app's biggest bottlenecks is millions of calls to cmlNormal, so yeah little things can matter. Sorry if I upset anyone! I just found it kind of surprising in a math lib.

…

On Tue, Jul 11, 2023 at 8:46 AM Tom MacWright ***@***.***> wrote: Okay, I'm happy to accept a PR if you want to write one, to improve performance here. I will say - I'm specifically talking about identifying toFixed as a bottleneck in a real-world program. Microbenchmarks can identify differences between Math.round versus the ~~ bit-twiddling hack, but 98% of those who care about that, including me in a previous iteration, are wrong to focus on it - the difference never rises to 1% of total-application performance budget, and usually gets optimized away once some V8 engineer decides to make Math.random or toFixed or etc faster. I will say, I'm really happy to discuss this stuff, but in the future - this discussion started with an incorrect assumption based on a misreading of the code, led to a question answered by a comment in the code, and now is about microbenchmarking. It would be certainly nicer and more productive to have fewer assumptions a generally more collaborative intent. — Reply to this email directly, view it on GitHub <#716 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APDPBBPQQINZBAQNWLV4Y2DXPVYN5ANCNFSM6AAAAAA2FKLK2Q> . You are receiving this because you authored the thread.Message ID: ***@***.***>

from simple-statistics.

why is a math lib returning strings? about simple-statistics HOT 7 CLOSED

Comments (7)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	return (
	Math.round((0.5 + (sum / SQRT_2PI) * Math.exp((-z * z) / 2)) * 1e4) /
	1e4
	);

	// The index we calculate must be in the table as a positive value,
	// but we still pay attention to whether the input is positive
	// or negative, and flip the output value as a last step.
	if (z >= 0) {
	return standardNormalTable[index];
	} else {
	// due to floating-point arithmetic, values in the table with
	// 4 significant figures can nevertheless end up as repeating
	// fractions when they're computed here.
	return +(1 - standardNormalTable[index]).toFixed(4);
	}