Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Robust rate expressions #100

Merged
merged 4 commits into from
Mar 17, 2024

Conversation

mopichalova
Copy link
Collaborator

@mopichalova mopichalova commented Mar 12, 2024

This update aims to close #87, which highlighted the limitations in representing complex rational expressions. Previously, the grammar supported only basic rational expressions without the capability for nesting divisions.

Only a small change has been made, by moving the fun "/" fun rule from being an exclusive part of the !rate rule to being incorporated within the !fun rule itself construction of more complex fractions and rational expressions is enabled.

Extensive tests have been added to cover the new grammar capabilities, ensuring that nested divisions are handled correctly and that the change does not introduce any regressions.

Copy link
Collaborator

@xtrojak xtrojak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you found any theoretical studies describing rational function expressions? It seems like every function allowed by this grammar should be convertible to a fraction of two polynomials, but is it really the case? Maybe I'm missing some mathematical fundamentals, but somehow, I feel like I need proof. 😃

@mopichalova
Copy link
Collaborator Author

Have you found any theoretical studies describing rational function expressions? It seems like every function allowed by this grammar should be convertible to a fraction of two polynomials, but is it really the case? Maybe I'm missing some mathematical fundamentals, but somehow, I feel like I need proof. 😃

Well, I relied on my mathematical common sense and that you would point out if it was insufficient 😄 I also feel like I am missing something and especially because it was just a small change.

But based on these articles from online Encyclopaedia of Mathematics: Rational function and Polynomials. The first article states that rational expression is essentially a fraction of two polynomials. And the second states that polynomial is a mathematical expression that involves variables raised to whole number powers and coefficients. I think all these operations should be covered in the grammar.

@xtrojak
Copy link
Collaborator

xtrojak commented Mar 15, 2024

@daemontus what do you think about this? Any insight on "rational expressions"?

@daemontus
Copy link
Member

daemontus commented Mar 15, 2024

I think this is essentially the correct conclusion. Any function that uses arbitrary combinations of operators *, /, +, - plus integer exponentiation can be transformed into a single fraction with a polynomial denominator and numerator (i.e. a rational function). But I also don't have a reference for this if that is what you are asking. I definitely read it somewhere though :D

The only issue is that this "canonical representation" using two polynomials is often quite large, but that shouldn't be relevant here.

Basically, the rationale behind the conversion is that without the / operation, these are clearly just polynomials that you can turn into the canonical form by propagating the multiplication and exponentiation down to the variables. Then, once you add /, you can use the lowest-common-denominator over polynomials to merge X1/X2 + X3/X4 into a single fraction and thus gradually extract the division such that you have only one big fraction.

Copy link
Collaborator

@xtrojak xtrojak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright then, thanks a lot for the comment @daemontus !
Let's merge it then @mopichalova, amazing job!

@xtrojak xtrojak merged commit 0b3ccae into sybila:improve-parsing Mar 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants