beltoforion / muparserx

A C++ Library for Parsing Expressions with Strings, Complex Numbers, Vectors, Matrices and more.
http://beltoforion.de/en/muparserx
BSD 2-Clause "Simplified" License
135 stars 60 forks source link

Large(ish) integers not supported #36

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. The following code throws:
mup::ParserX parser(mup::pckALL_NON_COMPLEX);
mup::Value value1{0};
mup::Variable variable1{&value1};
parser.DefineVar("v1", variable1);
parser.SetExpr("2147483648");
try {
    cout << parser.Eval().GetInteger() << endl;
} ...

What is the expected output? What do you see instead?

The value is only 2^31, shouldn't values up to 2^64-1 be supported for unsigned 
(long long) integers? Is it possible to specify the type of the variable at 
compile-time (and get a parsing error if it is not used as such in the 
expression)?

Currently I'm planning to use std::enable_if<std::is_integral<... and 
enable_if<is_floating_point<... to use either parser.Eval().GetInteger() or 
parser.Eval().GetFloatingpoint() to return the correct value from the parser. 
It would be nice if all types if integers and floating points were supported...

What version of the product are you using? On what operating system?

3.0.1

Original issue reported on code.google.com by ilja.j.h...@nasa.gov on 14 Jul 2014 at 2:49

GoogleCodeExporter commented 9 years ago
Tests indicate that 2^31 is interpreted as a floating point value. You can use 
it in computations but you can not retrieve it with GetInteger(). This is a 
general problem since muparserx does not differentiate between double and 
integer values internally. I havn't made up my mind what to do here since it 
affects only the GetInteger function. I may end up removing GetInteger 
altogether. Maybe i merge GetInteger and GetFloat into a GetValue function.

Original comment by ib...@gmx.info on 14 Jul 2014 at 8:48

GoogleCodeExporter commented 9 years ago
So internally muparserx always uses doubles? If there is no way to change that 
I vote for removing GetInteger. Are integers in expressions always converted to 
doubles for computations or are expressions computed with infinite precision? 
If the internal representation is always double that'd result in loss of 
precision when using large enough integers with non-zero least significant 
digits (larger than DBL_MANT_DIG or DBL_DIG?).

Original comment by ilja.j.h...@nasa.gov on 15 Jul 2014 at 2:42

GoogleCodeExporter commented 9 years ago
Internally muparserx is using std::complex<double> as its base type. Integers 
are always mapped to double. You will see loss of precision in integer values 
whenever the number of digits in your integer exceeds the number of digits 
returned by std::nuneric_limits<double>::digits10. You should be fine for up to 
48 bit integer values. 

Original comment by ib...@gmx.info on 15 Jul 2014 at 4:31

GoogleCodeExporter commented 9 years ago
I would also like to see support for large integers int64_t. I was about to try
recompiling muparserx and redefine the int typedef to int64_t, but the comments 
above
make me think this is not going work.

Original comment by Victor.W...@gmail.com on 24 Jan 2015 at 3:28

GoogleCodeExporter commented 9 years ago
Changed the typedef of int to in64_t. This exposed the need for a lot of 
explicit
casting to (int_type) in several library files and a lot in the unit tests.

Had to switch from clang to gcc to avoid a problem with use of undefined 
__float128.
Everything compiled file. The example mostly succeeds except for a couple of 
matrix tests.

Attached is the changed source code and the output from the example showing the 
unit test results.

Original comment by Victor.W...@gmail.com on 26 Jan 2015 at 3:46

Attachments:

GoogleCodeExporter commented 9 years ago
I doubt the unit tests are sufficient now since we need to test integer values 
at the std::limit<int_type>::max() and min()

Original comment by Victor.W...@gmail.com on 26 Jan 2015 at 3:49

GoogleCodeExporter commented 9 years ago
I am eager to see support for large integers

Original comment by Jeffrey....@gmail.com on 26 Feb 2015 at 2:50

jeffreyscottgraham commented 9 years ago

Is there a plan to address this issue?

beltoforion commented 9 years ago

When working with large integer number you will receive an overflow exception as soon as the number of digits in the number exceed 15. So internally you'll have 48 bits to work with your integers. Giving you more is a problem. I can only raise proper errors if that happens.

Externally calling GetInteger will only return a 32 bit integer. Thats inconsistent. Maybe i'll remove GetInteger() altogether. I havn't decided yet

nasailja commented 8 years ago

If the internal type is always or will always be double or complex double then I vote for removing getinteger(). In safe cases I guess double will be converted to int when needed and otherwise the compiler will warn or give an error in which case the value can be converted explicitly without getinteger().

jeffreyscottgraham commented 8 years ago

I have a commercial product that uses the 3x version, and cannot move to the latest version because integer support was removed. Please put it back.

beltoforion commented 8 years ago

I don't understand that. the change affected the API only. "Integer support" was always only a label rather then somthing that was done internally. It was a mistake to have it in the first place and a major source of confusion. I just removed value construction from integer. 4.x will work in your code if you cast your integers to a float when creating a variable. Every expression that was computed in 3.x should also compute in 4.x. (Notable exception: Casting to integer)

As far as returning results with GetInteger is concerned i'm currently thinking that this might be usefull in case you have expressions that are supposed to be dealing with integer values exclusively since it will just check the return value for integer properties and return it as such or throw an exception otherwise.

guruofquality commented 8 years ago

So I had a small amount of code that broke because of the API change, though it wasnt hard to work around. To me, the integer constructor added a sense of completeness and a way to express the intention "this variable in an integer".

Although I understand that the underlying implementation throws out that intention and just stores a double. So I can see where the confusion comes in. You are damned if you do, damned if you don't... :-P

It might be nice for users of the API to be able to express that intention, and perhaps one day, muparserx internals may may use of that extra piece of information that this variable is an integer (like you said).

jeffreyscottgraham commented 8 years ago

In version 3, my code did not need to care if the numerics were floating point, integer,boolean or string, and met requirements perfectly. In version 4, I need to know if an expression is a pure integer expression or mixed with floating point values too, and worry about precision as well. IMNSHO, that is a major function and responsibility of the library itself and a user should not need to deal with it

beltoforion commented 8 years ago

Check out 4.0.5 if this is sovling your problem.

jeffreyscottgraham commented 8 years ago

4.0.5 works great! Thanks for restoring integers!

Ingo Berg notifications@github.com wrote:

Check out 4.0.5 if this is sovling your problem.

— Reply to this email directly or view it on GitHub.