Mages core produce incorrect output

[htcfreek]

There is a bug in mages that results in incorrect output. Even if there is nothing to calculat. It happens with decimal and int numbers.

The following is happening:

Originally posted by @htcfreek in microsoft/PowerToys#18574 (comment)

[FlorianRappl]

The number is too long and cannot be stored in 64-bit integer.

The solution would be to only use 64-bit integer for "smaller" numbers and accept the loss of accuracy beyond (going to doubles). Going immediately to doubles would not be a good idea.

Happy to accept PRs on this one.

Lines in question are 113 an 181 (https://github.com/FlorianRappl/Mages/blob/devel/src/Mages.Core/Tokens/NumberTokenizer.cs#L113).

[htcfreek]

@dakersnar
Can you maybe take a look on this as you may have more insights in this mathematics than I. This is a bit out of my knowledge scoope.

[htcfreek]

The number is too long and cannot be stored in 64-bit integer.

The solution would be to only use 64-bit integer for "smaller" numbers and accept the loss of accuracy beyond (going to doubles). Going immediately to doubles would not be a good idea.

Happy to accept PRs on this one.

Lines in question are 113 an 181 (https://github.com/FlorianRappl/Mages/blob/devel/src/Mages.Core/Tokens/NumberTokenizer.cs#L113).

@FlorianRappl
Would it be possible to switch from uint64 to BigInteger?
Wait. Is it correct that you convert Uint64 to double?

[FlorianRappl]

Yes that could be another option. I'd need to see what the perf impact is either way.

[htcfreek]

Yes that could be another option. I'd need to see what the perf impact is either way.

FYI, .net 7 may come with int128 and uint128.

[htcfreek]

It might be good to prevent parsing nunber ls that will be to large and throw an exception in that case.

[FlorianRappl]

It might be good to prevent parsing nunber ls that will be to large and throw an exception in that case.

We don't throw errors - as we don't compile this might be a runtime error and is therefore very destructive. We could just set the value to NaN in such cases.

FYI, .net 7 may come with int128 and uint128.

Yeah I don't think taking more fixed bytes solves the problem. Then the number from OP works, but some user will always come up with a larger input / number. As we anyway use doubles internally we can safely transition to losing precision after the 64-bit range is exceeded.

[htcfreek]

It might be good to prevent parsing nunber ls that will be to large and throw an exception in that case.

We don't throw errors - as we don't compile this might be a runtime error and is therefore very destructive. We could just set the value to NaN in such cases.

I thought on something like Mages.Core.ParseException with an error message like this: "One of the numbers to parse is bigger than allow." This is better than returning invalid (don't mean loosing presicion) results.

[FlorianRappl]

Other scripting languages such as JavaScript also don't throw errors in these situations. They just lose precision.

Throwing a parse exception is rather destructive and only makes sense if the parsed content cannot be understood. This is not the case here. The parser understands it - it's just that the desired precision does not match the computing abilities.

[FlorianRappl]

Fixed (in 2.0.1) with digit shifting after perf. evaluation.

Original:

Method	Mean	Error	StdDev
Mages_AddTwoNumbers	1.736 us	0.0164 us	0.0153 us
Mages_AddMultiplyDivideAndPowerNumbers	6.086 us	0.0550 us	0.0514 us
Mages_MultiplyTwoVariables	4.668 us	0.0421 us	0.0394 us
Mages_CallStandardFunctions	9.480 us	0.0529 us	0.0495 us
Mages_Transpose4x5Matrix	18.074 us	0.1554 us	0.1377 us

BigInteger:

Method	Mean	Error	StdDev
Mages_AddTwoNumbers	1.932 us	0.0065 us	0.0061 us
Mages_AddMultiplyDivideAndPowerNumbers	6.797 us	0.0188 us	0.0176 us
Mages_MultiplyTwoVariables	4.829 us	0.0192 us	0.0160 us
Mages_CallStandardFunctions	9.973 us	0.0411 us	0.0343 us
Mages_Transpose4x5Matrix	20.871 us	0.1552 us	0.1452 us

Digit Shifting:

Method	Mean	Error	StdDev
Mages_AddTwoNumbers	1.734 us	0.0047 us	0.0044 us
Mages_AddMultiplyDivideAndPowerNumbers	6.069 us	0.0270 us	0.0253 us
Mages_MultiplyTwoVariables	4.693 us	0.0339 us	0.0317 us
Mages_CallStandardFunctions	9.541 us	0.0298 us	0.0264 us
Mages_Transpose4x5Matrix	18.469 us	0.0979 us	0.0868 us

[htcfreek]

Fixed (in 2.0.1) with digit shifting after perf. evaluation.

Original:

Method	Mean	Error	StdDev
Mages_AddTwoNumbers	1.736 us	0.0164 us	0.0153 us
Mages_AddMultiplyDivideAndPowerNumbers	6.086 us	0.0550 us	0.0514 us
Mages_MultiplyTwoVariables	4.668 us	0.0421 us	0.0394 us
Mages_CallStandardFunctions	9.480 us	0.0529 us	0.0495 us
Mages_Transpose4x5Matrix	18.074 us	0.1554 us	0.1377 us

BigInteger:

Method	Mean	Error	StdDev
Mages_AddTwoNumbers	1.932 us	0.0065 us	0.0061 us
Mages_AddMultiplyDivideAndPowerNumbers	6.797 us	0.0188 us	0.0176 us
Mages_MultiplyTwoVariables	4.829 us	0.0192 us	0.0160 us
Mages_CallStandardFunctions	9.973 us	0.0411 us	0.0343 us
Mages_Transpose4x5Matrix	20.871 us	0.1552 us	0.1452 us

Digit Shifting:

Method	Mean	Error	StdDev
Mages_AddTwoNumbers	1.734 us	0.0047 us	0.0044 us
Mages_AddMultiplyDivideAndPowerNumbers	6.069 us	0.0270 us	0.0253 us
Mages_MultiplyTwoVariables	4.693 us	0.0339 us	0.0317 us
Mages_CallStandardFunctions	9.541 us	0.0298 us	0.0264 us
Mages_Transpose4x5Matrix	18.469 us	0.0979 us	0.0868 us

How are these tables meant? Wath does this numbers mean?

When will v2.0.1 be available?

[FlorianRappl]

How are these tables meant? Wath does this numbers mean?

Just time to run certain perf. tests that we have. Since you are not familiar with the code / project they should not matter much to you. Lower (i.e., shorter time) is faster.

When will v2.0.1 be available?

Today.