ncalc / ncalc

Mathematical Expressions Evaluator for .NET
https://ncalc.github.io/ncalc/
MIT License
526 stars 75 forks source link

StackOverflow Exception on Evaluate function #167

Closed PuneetG123 closed 1 week ago

PuneetG123 commented 1 month ago

I am getting StackOverflow exception on evaluate function and I guess this is because of long precision numbers not sure though as the exception doesn't have much details in it whereas the same evaluation was working fine on NCalc 3.10.0 version.

Bykiev commented 1 month ago

Hi, can you please show your expression?

PuneetG123 commented 1 month ago

Hi Below is my expression: ((((A (B / 4)) + ((1 - A) (C / 8))) * D) + E) Where A, B, C, D, E are considered as array of length 300. I am using ExpressionOptions.IterateParamter option during evaluation.

gumbarros commented 1 month ago

@PuneetG123 good morning,

Can you send a snippet setting the parameters and evaluating so I can write a unit test and fix?

PuneetG123 commented 1 month ago

Good Morning,

Below is the code snippet that I am using to set the parameters:

for (int i = 0; i < iTotalNumRecords; i++) { int counter = 0; Expression e = new Expression(expText, EvaluateOptions.IterateParameters); e.EvaluateFunction += EvaluateFunctionExpression; foreach (string str in lArguments) {

List<double> d1 = new List<double>(lArguments1[counter][i].Replace("{", "").Replace("}", "").Split(',').Select(double.Parse).ToList());
e.Parameters.Add(str, d1);

counter++;
}
var d2 =  (object)e.Evaluate();
string d3 = "{" + string.Join(",", ((List<object>)d2).ToArray()) + "}";
lResult.Add(d3);

}

In the above code there are 2 lists

  1. lArgments : Contains the name of the of parameters like inthe above expression it is A,B, C etc..
  2. lArguments1 : Contains the value corresponds to that parameter. Which is also a list of numeric values.

Please let me know if you need anything else from my side.

Thanks

gumbarros commented 1 month ago

Good Morning,

Below is the code snippet that I am using to set the parameters:

for (int i = 0; i < iTotalNumRecords; i++) { int counter = 0; Expression e = new Expression(expText, EvaluateOptions.IterateParameters); e.EvaluateFunction += EvaluateFunctionExpression; foreach (string str in lArguments) {

List<double> d1 = new List<double>(lArguments1[counter][i].Replace("{", "").Replace("}", "").Split(',').Select(double.Parse).ToList());
e.Parameters.Add(str, d1);

counter++;
}
var d2 =  (object)e.Evaluate();
string d3 = "{" + string.Join(",", ((List<object>)d2).ToArray()) + "}";
lResult.Add(d3);

}

In the above code there are 2 lists

  1. lArgments : Contains the name of the of parameters like inthe above expression it is A,B, C etc..
  2. lArguments1 : Contains the value corresponds to that parameter. Which is also a list of numeric values.

Please let me know if you need anything else from my side.

Thanks

I think you need to change:

string d3 = "{" + string.Join(",", ((List<object>)d2).ToArray()) + "}";

to

string d3 = "{" + string.Join(",", ((List<object>)d2.Evalute()).ToArray()) + "}";

Because this would cause a infinite recursive loop, I think, help from outside is appreciated.

Beckdotan commented 2 weeks ago

I also have stackOverFlow exception problem when I deal with arrays. but since I have my own visitor I'm not using the .Evaluate() syntax at all.

Full description described in issue 192, but i also noticed that I get that only when I use '()' in my expression.

gumbarros commented 2 weeks ago

I also have stackOverFlow exception problem when I deal with arrays. but since I have my own visitor I'm not using the .Evaluate() syntax at all.

Full description described in issue 192, but i also noticed that I get that only when I use '()' in my expression.

Can you share the stacktrace of the exception? So we can check if it's a parser exception or a custom evaluation exception

Beckdotan commented 2 weeks ago

@gumbarros Thanks!

the stacktrace seems to be "null"

image

But i might have another clue:

Despite the fact I'm using .net framework 4.8 in my project, this part of the code in Ncalc is bright, and not the "under 6.0".

image

in my project I add the NCalc project manually (and no thought NuGet) so I can see what is going on.

this is the settings of it:

image

but it does stop there:

image
gumbarros commented 2 weeks ago

@gumbarros Thanks!

the stacktrace seems to be "null" image

But i might have another clue:

Despite the fact I'm using .net framework 4.8 in my project, this part of the code in Ncalc is bright, and not the "under 6.0". image

in my project I add the NCalc project manually (and no thought NuGet) so I can see what is going on.

this is the settings of it: image

but it does stop there: image

Try removing .Compile

Beckdotan commented 1 week ago

@gumbarros

Thanks. Can you explain what this .compile does? since when I delete it, it seems to be working fine, so I'm wondering if it is necessary at all. But I dont see any calculation time differences, that way.

I'm waiting for updating for a verry long time to get the running time benefits from all your amazing hard work!

Please also have a look at the other bug i opened there is full details there as well

Bykiev commented 1 week ago

From the Parlot docs:

Grammar trees built using the Fluent API can optionally be compiled with the Compile() method. At that point, instead of evaluating recursively all the parsers in the grammar tree, these are converted to a more linear and optimized but equivalent compiled IL. This can improve the performance by 20% (see benchmarks results).

If it's working without compilation then you should create an issue in Parlot repo.

Beckdotan commented 1 week ago

@Bykiev I feel like we are going in circles, since I already did all this tests in the other bug I opened, I want told to open a bug in parlot, which they closed saying that they are following on my bug in this repository.

@Bykiev @gumbarros Thanks for the information.

@sebastienros Since I saw you closed my bug in the parlot repo, can you please let me know how do you want to proceed from here? whether in this bug or in my original one in ncalc? or do you want me to open new bug in parlot?

sebastienros commented 1 week ago

@Beckdotan In theory if it works without compile I am pretty sure this could be an issue in Parlot, but here we have some code that we can work with, we have nothing at all to work with in the Parlot repository. Once I know what the issue is "with Parlot" then we can add an issue there.

Back to your issue (the other one), until you can provide a way to reproduce the problem then we won't be able to help. And even more, and it would be very helpful if you could help us repro the problem in a unit test that is minimal, not involving so many components. Based on your original comment it should be as simple as executing the expression you shared, but it doesn't look like so, and AFAIR as you were not able to create some code that fails outside of your own application. At least please add more of the stacktrace that shows where the actual overflow happens (to exclude one coming from your own custom functions).

General comment about Compile():

Instead of evaluating a bunch of conditions to drive what has to be done on each execution, Parlot can decide which code to execute once and create an Expression Tree that is then compiled. Another example is when SkipAnd is called, it know the value can be discarded, so it won't try to materialize a value, more like pattern matching. In the end it's makes parsing faster. All the tests in Parlot are executed with and without compilation to ensure it works but there might be gaps so it's not bug-free, either way.

sebastienros commented 1 week ago

I think we should close this local issue as there was not answer from the author, the suggestion might have worked.

@Beckdotan your issue might be totally different, we should continue on the other issue you created. I don't see any reason to think it's the same SO exception.