quantulum3 icon indicating copy to clipboard operation
quantulum3 copied to clipboard

fix for 212 (WIP)

Open yoavg opened this issue 3 years ago • 6 comments

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 avatar Jan 07 '23 17:01 yoavg

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 avatar Jan 08 '23 07:01 yoavg

? @nielstron

yoavg avatar Jan 11 '23 21:01 yoavg

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

nielstron avatar Jan 12 '23 18:01 nielstron

no problem at all, we are all busy :)

On Thu, Jan 12, 2023, 20:27 Niels Mündler @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/nielstron/quantulum3/pull/213#issuecomment-1380829911, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADRLNBDET7ETH7JOHWPX6TWSBEKNANCNFSM6AAAAAATUCJF4A . You are receiving this because you authored the thread.Message ID: @.***>

yoavg avatar Jan 12 '23 18:01 yoavg

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.

nielstron avatar Jan 16 '23 16:01 nielstron

thanks, Yes, this makes sense! But I do think that if an operator is explicit, we should allow it also if surrounded by space. I will try to make it work.

Re the ordering fix: yes, this is indeed because of sets, and I have a fix for that in the PR, but unfortunately it fixed things to be consistent on the wrong result :( and I think that a solution that depends on the order in the list of operators is too brittle, so looking for something better.

On Mon, Jan 16, 2023 at 6:33 PM Niels Mündler @.***> wrote:

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.

— Reply to this email directly, view it on GitHub https://github.com/nielstron/quantulum3/pull/213#issuecomment-1384297709, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADRLNCHSX4FMV7LI7XCRZDWSVZ37ANCNFSM6AAAAAATUCJF4A . You are receiving this because you authored the thread.Message ID: @.***>

yoavg avatar Jan 16 '23 17:01 yoavg