ParserNG
ParserNG copied to clipboard
Add bigdecimal capability
This PR exposes bigdecimal arithmetic features to ParserNG users. This is the first stage of this feature.
For normal floating point arithmetic, we use the MathExpression
class.
For bigdecimal arithmetic, the active class is the BigMathExpression
class.
What is supported
- BigDecimal arithmetic operations on numbers and variables
- BigDecimal arithmetic operations on functions that are composed only of numbers and variables and other such functions that can be broken down to numbers and variables alone.
What is not supported yet
- BigDecimal arithmetic operations on inbuilt functions
- BigDecimal arithmetic operations on user defined functions that include inbuilt functions
- Logical expressions are not enabled yet(feature disabled), in contrast to the MathExpression class which handles boolean expressions easily.
Still to come
- Further updates as more functions that support bigdecimal arithmetic are contributed
So this will work:
BigMathExpression bme = new BigMathExpression("x=2;h(x)=3*x^2+x+2*x;h(3);");
System.out.println(bme.solve());
But this wont:
BigMathExpression bme = new BigMathExpression("x=2;h(x)=3*x^2+x+2*x-sin(x);h(3);");
System.out.println(bme.solve());
The last expression, h(3)
will fail at runtime because it contains sin(x)
(an inbuilt function) in its definition, whose BigDecimal
implementation is non-existent.
"Will fail at runtime" implies that the function: h(x)=3*x^2+x+2*x-sin(x)
will be successfully compiled and stored in parser memory, but when we try to use it by running h(3)
, it will give an error
@judovana. Please check
Ugh. This is very hard to read. It seems to be mixing various reformatting and refactoring together with logical changes. I guess youa re not willing to split those to individual commits?
Permanent usage of main methods as testing area with various commented out/in features - unittests are here fort his.
Generally speaking thei feature lack unittests at all - mainly it lacks targetted unitteest for the touched coede.
Missing unittests for new logic.
Various irrelevant outputs to stdout. If yuu really need the,m the verbose mode should be added. While it is not properly here, they should be printed out to stderr
The usage of return true;//contraband
is very error prone, should ber hadled by BigMathExpression
design, or exception should be thrown if it is not honoured.
The main problem remains the huge duplcicated code, mainly in BigMathExpression
. Such thing bites.
If you would be my intern all above will be show stoppers for merging
- do the refactoring in seaprate commit (maybe refactor whole parser ng in one huge comment?)
- do the cosmetic changes (eg changes in comments) in separate commit
- until this is in, no more reviews
- implement logging/debuggin/tracing logic
- add unittests instead of main methods
- add unittests targetting the feature
- although it can maybe go in with duplcated code, have a plan how to get ird of it before push
- only having the plan may be enough for not letting the duplicated code in, because the plan to get rid of it will be good
But you are not my intern, and this is yours project :) So only good advices can be given.
One political request if I can. I have PR adding docs for expanding and logical parsers and bumpiong ParserNG to 0.1.9. As this PR may be possibly breaking change, do you mind to release 1.9 before continuing with feature ?
Ugh. This is very hard to read. It seems to be mixing various reformatting and refactoring together with logical changes. I guess youa re not willing to split those to individual commits? Permanent usage of main methods as testing area with various commented out/in features - unittests are here fort his. Generally speaking thei feature lack unittests at all - mainly it lacks targetted unitteest for the touched coede. Missing unittests for new logic. Various irrelevant outputs to stdout. If yuu really need the,m the verbose mode should be added. While it is not properly here, they should be printed out to stderr The usage of
return true;//contraband
is very error prone, should ber hadled byBigMathExpression
design, or exception should be thrown if it is not honoured. The main problem remains the huge duplcicated code, mainly inBigMathExpression
. Such thing bites.If you would be my intern all above will be show stoppers for merging
do the refactoring in seaprate commit (maybe refactor whole parser ng in one huge comment?)
do the cosmetic changes (eg changes in comments) in separate commit
- until this is in, no more reviews
implement logging/debuggin/tracing logic
add unittests instead of main methods
add unittests targetting the feature
although it can maybe go in with duplcated code, have a plan how to get ird of it before push
- only having the plan may be enough for not letting the duplicated code in, because the plan to get rid of it will be good
But you are not my intern, and this is yours project :) So only good advices can be given.
One political request if I can. I have PR adding docs for expanding and logical parsers and bumping ParserNG to 0.1.9. As this PR may be possibly breaking change, do you mind to release 1.9 before continuing with feature ?
lol, sorry.. I created ParserNG in 2009. So well, a lot of design decisions taken then would not fly now. I was more concerned about creating the parser and making it multi-featured then.
One political request if I can. I have PR adding docs for expanding and logical parsers and bumpiong ParserNG to 0.1.9. As this PR may be possibly breaking change, do you mind to release 1.9 before continuing with feature ?
Send in the PR
The usage of
return true;//contraband
is very error prone, should ber hadled byBigMathExpression
design, or exception should be thrown if it is not honoured.
The method is marked private
and the exception is thrown in the BigMathExpression
constructor or am I missing something?
Permanent usage of main methods as testing area with various commented out/in features - unittests are here fort his. Generally speaking thei feature lack unittests at all - mainly it lacks targetted unitteest for the touched coede. Missing unittests for new logic.
True. Will add unit tests where relevant, going forward. Thanks
The usage of
return true;//contraband
is very error prone, should ber hadled byBigMathExpression
design, or exception should be thrown if it is not honoured.The method is marked
private
and the exception is thrown in theBigMathExpression
constructor or am I missing something?
I guess no. More likely I may be missing something. Can you point up a line please>
lol, sorry.. I created ParserNG in 2009. So well, a lot of design decisions taken then would not fly now. I was more concerned about creating the parser and making it multi-featured then.
No need to apologise. Still parseng is one of the younger. Also it is indeed feature-full. Still the tests are now there, and should be used right away O:)
ps: mixing refactroign reformating and logical changes is one of the most terrible evils in sowftare development
Still, thanx a lot for working on this!
I would still heavil vote for heavy rework of initialcommits - to split refactorings, cosemtic changes, a and reallogic change. If this is done (then the logic change will be reviwable) then soem effort how to remove duplicated code shoudl be done ( I will be happy to help)
Tahnx!
Jsut for small info - it wil be easy to adapt main CLI to use this new BIgDecimal parser. Also all "underlying" parsers (like expandable and logical) are already parametrised so they can work with BigDecimalMath out of the box. Great work. TYVM!
Hello!
I'm running set of JMH benchmarks - basic double
x Double
x BigDecimal
toString/fromString and arithemtical operations. and advanced, with ParserNG MathExpression
double x BigMathExpression
BigDecimal implemenataion and also on a bit rush DoubleMatheExpression
on Double.
It is currently moreover showing that double Double is practically no difference. Which should lead this PR to nearly complete removal double, and making MathExpression
easily extendable as BigDecimal, Float, or generally any Number.
I hope to have some publishable results to the end of next week.
In meantime... Maybe 0.1.9 can be released. In meantime... all the refactorings and cosmetic changes can be pushed, so this PR will be clear and clean?
Ok, for now, just to String and from string:
https://github.com/judovana/CustomJmhBenchmarks/tree/numbers
It is interesting. It seems that it should move directly to BigDecimal :)
float < BigDecimal << double
and
float == Float and double == Double
So in the autoboxing, java is doing really great work. I am looking forward to arithmetic measurements and to ParserNG itself. Not sure when I will have more time :(
now some basic math:
https://github.com/judovana/CustomJmhBenchmarks/blob/numbers/README.md#basic-math
looks more like expected . More autoboxing in equation, the worse. However it is still very minor, and generally double=Double=float=Float
The BigDecimal however, is here super slow (which is logical, as it do not count in cpu but algorithmically)
MathExpression x BigMathExpression:
https://github.com/judovana/CustomJmhBenchmarks/blob/numbers/README.md#mathexpression-x-bigmathexpression
In the complexity of parser, the slowdown of the move from double
to BigDecimal
is nealry nothing.
I'm planning to add also include DoubleMathExpression - jsut for this meassurments - but from all we see, the impact of double
x Double
is nothing. Impact of double
x BigDecimal
is nearly nothing.
Thus saying, the work should be done to make MathExpression properly extend-able, And thus have no duplicated code nowhere and support in functions out of the box.
Originally I wanted to write some DoubleMathExpression as you did in BigMathExpression, but then I chaged my mind and simply converted whole ParserNG from double to Double -https://github.com/judovana/ParserNG/commit/352745f80d47333af28c5fce60a766c5c3d63b0b
Results are here: https://github.com/judovana/CustomJmhBenchmarks/blob/numbers/README.md#parserng-rewritten-from-double-to-double
interesting is, that whole ParserNG got nearly immeasurable, but still visible speed up.
That again is saying, that adding BigMathExpression should be done by heavy changes elsewhere, and BigMathParser should then be just candy on top of it all.
Note, that without support of functions, BigMathParser is useless, and should not go in. Functions are heartbeat of ParserNG. Thus saying, I'm against this PR (but I do not have any vote). At least the functions should be enabled as in fallback mode, so they do nto provide necessary precission, but are accessible.
As input to functions is String
- and I consider it as excelent - a converting factory must be somewhere provided. And as Number
interface do not enforce mult/div/add/subs function ... well, its hard task.
Unless you will require additional measurement, I think I'm finished with them.
Oh, one important nit. With the BigMathExpression
parser comes need of globally controlled MathContext
. See https://github.com/gbenroscience/ParserNG/blob/master/src/main/java/math/BigDecimalNthRootCalculation.java#L12
I already did it wrong, as I was really not forseeing BigMathExpression - and eg here: https://github.com/gbenroscience/ParserNG/blob/master/src/main/java/parser/methods/ext/Avg.java#L22 it is redeclared.
Originally I wanted to write some DoubleMathExpression as you did in BigMathExpression, but then I chaged my mind and simply converted whole ParserNG from double to Double -judovana@352745f
Results are here: https://github.com/judovana/CustomJmhBenchmarks/blob/numbers/README.md#parserng-rewritten-from-double-to-double
interesting is, that whole ParserNG got nearly immeasurable, but still visible speed up.
That again is saying, that adding BigMathExpression should be done by heavy changes elsewhere, and BigMathParser should then be just candy on top of it all.
Note, that without support of functions, BigMathParser is useless, and should not go in. Functions are heartbeat of ParserNG. Thus saying, I'm against this PR (but I do not have any vote). At least the functions should be enabled as in fallback mode, so they do nto provide necessary precission, but are accessible.
As input to functions is
String
- and I consider it as excelent - a converting factory must be somewhere provided. And asNumber
interface do not enforce mult/div/add/subs function ... well, its hard task.Unless you will require additional measurement, I think I'm finished with them.
Thanks for all the work @judovana I apologize for being away for so long. Will do my best to find time to look at it all again, and also release 0.1.9
no worries sir :)
I believe the only missing piece is some class NumberProvider, which will be converting strings and numbers, and returning some Number on demand as needed. Then MathParser, FloatMathParser and BigMathParser, built on top of some GenericNumberProvidingParser should be easy enough.
As functions are taking array of Strings, they will need to handle on theirs own and support, or not support double/big/float inputs. Or the contract of functions will need to be changed, so they would receive already parsed numbers, and then the support will be just in overloading of some solve(<double/float/big> array) . However still many of implementations will need to be type-dependent similarly as the basic math operations above big x float/double