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

fix for 212 (WIP) #213

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

fix for 212 (WIP) #213

wants to merge 4 commits into from

Conversation

yoavg
Copy link
Contributor

@yoavg yoavg commented Jan 7, 2023

This is an attempt to fix #212

Now the parsing behavior is consistent, but we cannot parse correctly some of the new test examples.

@yoavg
Copy link
Contributor Author

yoavg commented Jan 8, 2023

I managed to get it to "work" in a somewhat convoluted way, but i still have some new test-cases that fail. they fail also in the current dev branch, though, e.g. 34kg*cm2 takes only the kg, while 34 kg*cm2 succeeds in taking both units.

There seem to be some logic in place directly for enforcing this behavior (or a related one) but I cannot figure out what's behind it. The comment (in parser.get_unit()) says "cut if inconsistent multiplication operator", can you elaborate on this notion of consistency?

@yoavg
Copy link
Contributor Author

yoavg commented Jan 11, 2023

? @nielstron

@nielstron
Copy link
Owner

sorry I have been quite busy this week and will maybe get to have a look I the coming days.

@yoavg
Copy link
Contributor Author

yoavg commented Jan 12, 2023 via email

@nielstron
Copy link
Owner

There seem to be some logic in place directly for enforcing this behavior (or a related one) but I cannot figure out what's behind it. The comment (in parser.get_unit()) says "cut if inconsistent multiplication operator", can you elaborate on this notion of consistency?

Finally got around to look at this. I think one issue when I picked up this project was that something like "kgm" would not be picked up as "kilogram meters" - because considering space a multiplication factor caused a lot of issues. One way to cope with this was that I just fixed "either all multiplications are explicit or all are implicit" - the way normal humans would write units. Something like "gml" (gramm meter litres) is super awkward - either it would be "gml" or "gm*l". Does that help you in any way?

I am wondering if the "different results" stem alone from the fact that operators are stored in sets and the order in which they are accessed may depend on their runtime hash ordering in the set - which might change on every reload of quantulum. This might be simply fixed by usings lists instead of sets to store them.

@yoavg
Copy link
Contributor Author

yoavg commented Jan 16, 2023 via email

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.

inconsistent parsing behavior
2 participants