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

Unexpected result of ge on -0.0 and 0.0 under magic rewrite #122

Closed
DerZc opened this issue May 19, 2023 · 10 comments
Closed

Unexpected result of ge on -0.0 and 0.0 under magic rewrite #122

DerZc opened this issue May 19, 2023 · 10 comments
Labels
bug Something isn't working Stale

Comments

@DerZc
Copy link

DerZc commented May 19, 2023

Hi,

Consider the following program:

phve[a, b] <- [[-0.0, null], [0.0, null]]
xukw[A, B] := phve[A, B], ge(A, 0)
nwku[F] := phve[_, F]

ssie[C, F] := nwku[C], xukw[F, C]
?[a, b] := ssie[a, b]

I get the results [[null, -0.0], [null, 0.0]], but the value of nwku is [[null]] and the value of xukw is [[0.0, null]], so the result of ssie should not be [null, -0.0].

I also tried to add :disable_magic_rewrite true, then I got correct results.

I can reproduce this on version 0.7.1

@zh217
Copy link
Contributor

zh217 commented May 19, 2023

Interesting, seems that this only happens with -0.0 and not with any other number.

@zh217 zh217 added the bug Something isn't working label May 19, 2023
@zh217
Copy link
Contributor

zh217 commented May 19, 2023

The gist of the problem can be summarized in:

?[] <- [[0.0 >= 0, -0.0 >= 0, -0.0 >= 0.0, 0.0 >= -0.0]]

The result is true, true, false, true in CozoDB. It looks like the second one shouldn't be true, but actually, for python, it is:

>>> 0.0 >= 0
True
>>> -0.0 >= 0
True
>>> -0.0 >= 0.0
True
>>> 0.0 >= -0.0
True

Not sure what can be done. Going the python way basically means that we will eliminate -0.0 from numbers, and I'm not sure that's what we want to do. Making the second one false as well means that we could mess up with the memcomparable format. Floating point numbers are hard.

At least throwing away -0.0 is self-consistent. So if everything else fails, maybe we will go the Python way.

@zh217
Copy link
Contributor

zh217 commented May 19, 2023

It seems Rust agrees with python:

assert!(0.0 >= -0.0);
assert!(-0.0 >= 0.0);

passes.

@DerZc
Copy link
Author

DerZc commented May 19, 2023

Hi @zh217 I have a small question. For this test case, is this problem caused by the transformation of ge(A, 0) to ge(A, 0.0) under the magic rewrite?

@zh217
Copy link
Contributor

zh217 commented May 19, 2023

No, magic rewrite doesn't change that. It is caused by magic rewrite enabling range scan on the relation phve instead of testing every value. Range scan uses the byte order of memcomparable, which in this case differs from the usual comparison in subtle ways.

@DerZc
Copy link
Author

DerZc commented May 19, 2023

I got it! Thank you very much!

@DerZc
Copy link
Author

DerZc commented May 19, 2023

I'm very sorry to disturb you. But I have another question. When does the magic set rewrite get triggered?
The following program should be equivalent to the this test case:

phve[a, b] <- [[-0.0, null], [0.0, null]]
xukw[A, B] := phve[A, B], ge(A, 0)
ssie[C, F] := phve[_, C], xukw[F, C]
?[a, b] := ssie[a, b]

But this program produces correct results, which means it not triggers range scan.

(By the way, there is a small error in the document of magic set rewrite https://docs.cozodb.org/en/latest/execution.html#magic-set-rewrites, the first line of the example should be reachable[a, b] := link[a, b], otherwise, b is unbound.)

@DerZc
Copy link
Author

DerZc commented May 22, 2023

Hi @zh217 so sorry to disturb you. Do you have documents available that outlines the specific case under which the magic set rewrite is applied? I just have the questions mentioned above.

@zh217
Copy link
Contributor

zh217 commented May 22, 2023

Sorry I haven't had time to look at the above behaviour yet. Magic set rewriting is a standard optimization technique for optimizing Datalog programs, and there are quite a few review articles describing it in details, the one easiest to read is probably Datalog and Recursive Query Processing. Our implementation follows their descriptions. Note that the rewriting technique only applies to pure Datalog, that is, without negation, aggregation, etc., so any time any of these are encountered they are left as it is. Once you know the terminologies of magic set rewriting, the result of ::explain {<query>} should make more sense.

@DerZc
Copy link
Author

DerZc commented May 22, 2023

I got it! Thank you very much for taking the time to respond to me amid your busy schedule!

@github-actions github-actions bot added the Stale label Jun 22, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Stale
Projects
None yet
Development

No branches or pull requests

2 participants