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

Edit intro based on reviews #134

Merged
merged 12 commits into from
Oct 31, 2023
Merged

Edit intro based on reviews #134

merged 12 commits into from
Oct 31, 2023

Conversation

loayshaqir1
Copy link
Collaborator

We edited the introduction.ipynb file based on the feedback we received from new people combined with Ahmad & Mahmoud's feedback.
Here are the list of questions/ things to clarify that we received:

  • Why when registering ie functions there is a yield statement instead of a simple return?
  • Did not understand how py_rgx_string outputs X,Y (where does it get them from?)
  • Missing example that shows the difference between py_rgx_string and py_rgx_span.
  • Explain how data is stored and when it gets computed, explain the lazy evaluation of rules.
  • Add more explanation to why we get an empty tuple when we query a relation that has no free variables (all constant) when the statement is true vs why we get nothing when its false.

So we edited the introduction file and added explanations to answer all of the above questions

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@loayshaqir1 loayshaqir1 requested a review from DeanLight October 30, 2023 08:52
@@ -89,147 +89,147 @@
"name": "stderr",
Copy link
Owner

@DeanLight DeanLight Oct 30, 2023

Choose a reason for hiding this comment

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

In theory, we usually say that a datalog/rgxlog query is trying to find all instantiations of the free variables that satisfy the query.

And this is why if we have a query with no free variables, we get an empty set of instantiations if its true and no such set if its false.

Perhaps you can present queries using the instantiation notation and then the explanation here will be easier to understand.


Reply via ReviewNB

@@ -89,147 +89,147 @@
"name": "stderr",
Copy link
Owner

@DeanLight DeanLight Oct 30, 2023

Choose a reason for hiding this comment

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

I dont think this "empty table" will persist in later versions, and are queries even saved in the DB?

What the users should know here is that we save the facts in the database, the rules' logic is saved seperately and only evaluated lazily (on demand).

One a query is issued, the engines uses the rules to derive all possible solutions from the existing facts that would satisfy the query.


Reply via ReviewNB

@@ -89,147 +89,147 @@
"name": "stderr",
Copy link
Owner

@DeanLight DeanLight Oct 30, 2023

Choose a reason for hiding this comment

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

why is this error message here?


Reply via ReviewNB

Copy link
Owner

@DeanLight DeanLight left a comment

Choose a reason for hiding this comment

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

added some comments

@loayshaqir1 loayshaqir1 requested a review from DeanLight October 30, 2023 10:35
@@ -85,152 +85,153 @@
"execution_count": null,
Copy link
Owner

@DeanLight DeanLight Oct 31, 2023

Choose a reason for hiding this comment

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

Why is this here? should we not install NLP in this case?


Reply via ReviewNB

@@ -85,152 +85,153 @@
"execution_count": null,
Copy link
Owner

@DeanLight DeanLight Oct 31, 2023

Choose a reason for hiding this comment

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

I would rewrite the second sentense "A query is essentially a way to retrieve specific information from a dataset, i.e it finds all instantiations of the free variables that satisfy the query. " the second part of the sentence is not an example of the first part, so the use of "ie" is incorrect.

Maybe just say that querying in rgxlog uses the same synatx and semantics as DataLog. Under said semantics, we try to find all instantiations of free variables that satisfy the queried relation.


Reply via ReviewNB

Copy link
Owner

@DeanLight DeanLight left a comment

Choose a reason for hiding this comment

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

address my 2 small comments and then you can merge,

@loayshaqir1 loayshaqir1 merged commit 1a6588e into master Oct 31, 2023
2 checks passed
@loayshaqir1 loayshaqir1 deleted the edit_intro_based_on_reviews branch October 31, 2023 10:31
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.

2 participants