Neos-Metaverse / NeosPublic

A public issue/wiki only repository for the NeosVR project
195 stars 9 forks source link

Support some kind of IsError node, especially for decimal-type overflows/div-by-zero #2219

Open RobertBaruch opened 3 years ago

RobertBaruch commented 3 years ago

Describe the bug

When working with the decimal type, NaNs and Infs are not supported. For example, a large decimal multiplied by a large decimal (in C#), the result is that a System.OverflowException is thrown. In logix, this causes an error condition.

The same happens when a decimal is divided by decimal zero: a System.DivideByZeroException is thrown. Also, when a decimal is divided by a very small decimal, the result could be that a System.OverflowException is thrown

In both cases, it would be nice to be able to detect this condition. For example, in a calculator application that uses decimals, it would be nice to beep if an operation on decimals overflows, rather than just halt execution, which looks like a bug.

Relevant issues

To Reproduce

Create a multiply node, with decimal inputs. Create a display output for the multiply node. Put a lot of 9s in the inputs. The display node will error. However, there is no way to detect this error in logix.

Expected behavior

Some kind of node that could attach to the multiply node to detect the error and output a bool.

Log Files

Screenshots / Video

Additional context

Reporters:

Xekri

shiftyscales commented 3 years ago

Do you have a screenshot or video of the reproduction steps? I can't seem to replicate this on my end. Additionally, it doesn't make sense to have error handling offered as a bool on these nodes, but rather to fix the underlying issue you're experiencing so it displays properly.

When I'm trying to reproduce it, it either goes to infinity, or rounds to the nearest value, both of which would be expected behavior. I can't see any 'error' on the display node.

If you want to detect infinity or nan, there are already IsInfinity and IsNaN nodes in Operators.

chemicalcrux commented 3 years ago

I've encountered similar problems from other nodes: an actual red output and the deletion of wires going to drive nodes. I encountered it with the Remap node when using the same values for the start and end range, iirc.

I can try to reproduce that in a little bit.

RobertBaruch commented 3 years ago

20210512124816_1

shiftyscales commented 3 years ago

Oh I see. It seems like the error handling for certain types might not operate the same as other types like floats(?)

https://user-images.githubusercontent.com/54213390/118035548-2d950b80-b320-11eb-867c-26df6fc15019.mp4

SHIFTYWINDOWS - 2021.5.9.179 - 2021-05-12 12_41_46.log

12:43:04 PM.214 ( 90 FPS)   Exception when Updating object: Element: IDD67500, Type: FrooxEngine.LogiX.Display.Display_Decimal, World: Shifty's Cloud Home, IsRemoved: False, IsDestroyed: False, IsDisposed: False, Enabled: True
Element: IDD65400, Type: FrooxEngine.Slot, World: Shifty's Cloud Home, IsRemoved: False, Slot name: Display, T: [-0.9765521; 1.054617; 2.345138], R: [3.10032E-06; 0.3758166; -1.2536E-06; 0.9266941], S: [0.6720005; 0.6720003; 0.6720001], ActiveSelf: True, IsDestroyed: False
Element: IDA8DD00, Type: FrooxEngine.Slot, World: Shifty's Cloud Home, IsRemoved: False, Slot name: Users, T: [0; 0; 0], R: [0; 0; 0; 1], S: [1; 1; 1], ActiveSelf: True, IsDestroyed: False
Element: ID17DF00, Type: FrooxEngine.Slot, World: Shifty's Cloud Home, IsRemoved: False, Slot name: World, T: [0; 0; 0], R: [0; 0; 0; 1], S: [1; 1; 1], ActiveSelf: True, IsDestroyed: False
Element: ID2000, Type: FrooxEngine.Slot, World: Shifty's Cloud Home, IsRemoved: False, Slot name: Root, T: [0; 0; 0], R: [0; 0; 0; 1], S: [1; 1; 1], ActiveSelf: True, IsDestroyed: False
Element: ID0, Type: FrooxEngine.World, World: Shifty's Cloud Home, IsRemoved: False

Exception:
System.OverflowException: Arithmetic operation resulted in an overflow.
  at (wrapper managed-to-native) System.Decimal.FCallMultiply(System.Decimal&,System.Decimal&)
  at System.Decimal.op_Multiply (System.Decimal d1, System.Decimal d2) [0x00000] in <9577ac7a62ef43179789031239ba8798>:0 
  at FrooxEngine.LogiX.Operators.Mul_Decimal.get_Content () [0x00020] in <082dfa144d5b4721a90ea99f0cc9d83a>:0 
  at FrooxEngine.LogiX.Input`1[T].EvaluateRaw (T def) [0x0000a] in <082dfa144d5b4721a90ea99f0cc9d83a>:0 
  at FrooxEngine.LogiX.Input`1[T].Evaluate (T def) [0x00010] in <082dfa144d5b4721a90ea99f0cc9d83a>:0 
  at FrooxEngine.LogiX.Display.Display_Decimal.OnChanges () [0x00009] in <082dfa144d5b4721a90ea99f0cc9d83a>:0 
  at FrooxEngine.ComponentBase`1[C].InternalRunApplyChanges (System.Int32 updateIndex) [0x00017] in <082dfa144d5b4721a90ea99f0cc9d83a>:0 
  at FrooxEngine.UpdateManager.ProcessChange (FrooxEngine.IUpdatable updatable) [0x00024] in <082dfa144d5b4721a90ea99f0cc9d83a>:0 
  at FrooxEngine.UpdateManager.RunQueue[T] (System.Collections.Generic.Queue`1[T] queue, System.Action`1[T] action) [0x00009] in <082dfa144d5b4721a90ea99f0cc9d83a>:0
Frooxius commented 3 years ago

We can only handle those cases by defaulting the output value to some default value when it happens, as you cannot output exception like that in the nodes.

Generally though I'd advise against using decimal datatype, as it's very special purpose one, mostly useful for financial calculations and as such you're not too likely to encounter these oddities. float and double should be used for pretty much everything unless you're dealing with financials (e.g. calculating NCR or KFC)

RobertBaruch commented 3 years ago

I originally chose decimals for the calculator app, thinking that the precision would be nice. But yeah, doubles are probably good enough. If I wanted a true scientific calculator, I'd probably just have to build something like GMP in logix :P

Feel free to close as Won't Fix.

Frooxius commented 3 years ago

Yeah it's not really suitable datatype for that. You can't do lots of common mathematical calculations on it either, it's mostly designed for financial calculations: https://docs.microsoft.com/en-us/dotnet/api/system.decimal?view=net-5.0

The main benefit is that it internally represents numbers in base 10, instead of base 2 like IEEE754 float types do, which prevents rounding errors, even when you're within the precision (e.g. value 0.3 is cannot be exactly represented in float/double, but can in decimal).

The rule of thumb is that if you're not doing financial calculations, use float/double.

chemicalcrux commented 3 years ago

Here's an example caused by the Multi Cosine Lerp node. All of the values involved are floats.

image

If I changed it to a Cosine Lerp node, it outputs NaN, rather than the weird broken state.