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

Add GIVEN * EXCEPT ... support #103

Merged
merged 2 commits into from
Apr 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion resources/inferenceql/query/permissive.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,15 @@ standalone-event-conjunction ::= identifier (ws #'(?i)AND' ws identifier)+

(* given *)

given-expr ::= model-expr ws #'(?i)GIVEN' ws (given-event-list | given-event-conjunction)
given-expr ::= model-expr ws #'(?i)GIVEN' ws (given-star-clause | given-event-list | given-event-conjunction)
given-event-list ::= given-event (ws? ',' ws? given-event)*
given-event-conjunction ::= given-event (ws #'(?i)AND' ws given-event)+

<given-star-clause> ::= star (ws? given-except-clause)?
given-except-clause ::= #'(?i)EXCEPT' (ws given-except-list | ws? <'('> ws? given-except-list ws? <')'>)
given-except-list ::= identifier ((ws? ',' ws? | ws #'(?i)AND' ws) identifier)*


<given-event> ::= (density-event-eq / distribution-event-binop)
| identifier

Expand Down
50 changes: 43 additions & 7 deletions src/inferenceql/query/permissive.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,46 @@
{:event event}))))

(defn identifier->variable
"Returns the variable node equivalent of an identifier node."
"Wraps an identifier node in a variable node."
[node]
[:variable "VAR" ws node])

(defn ^:private ands->commas
"Replaces subsequent pairs of whitespace nodes followed by \"AND\" with
a comma."
[node]
;; Really awkward to do this with a loop, but reduce/partition alternatives
;; have the problem of not being able to look ahead easily, or skip over
;; elements. An index is actually easier.
(let [cnt (count node)
stop-idx (dec cnt)]
(if (= 1 cnt)
node

Check warning on line 69 in src/inferenceql/query/permissive.cljc

View check run for this annotation

Codecov / codecov/patch

src/inferenceql/query/permissive.cljc#L69

Added line #L69 was not covered by tests
(loop [i 0
acc []]
(cond (> i stop-idx) acc ; final pair was ws+and, so we overshot
(= i stop-idx) (conj acc (peek node)) ; conj last elt
:else (let [n1 (nth node i)
n2 (nth node (inc i))]
(if (and (tree/whitespace? n1)
(string? n2)
(re-matches #"(?i)AND" n2))
(recur (+ 2 i) (conj acc ",")) ; skip to after AND
(recur (inc i) (conj acc n1)))))))))

Comment on lines +59 to +81
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If people think this is overkill, I can change or remove it. (Only real consequence is spaces before commas when unparsing the converted perm->strict query. E.g., model GIVEN * EXCEPT foo AND bar AND moop -> model CONDITIONED BY * EXCEPT foo , bar , moop.)

I know it's ugly, but the need to look at pairs, and skip over input makes it awkward. I considered variations using partition, reduce, rest/butlast, etc. and none of them turned out cleaner. Too many fns aren't designed to skip input or work with more than one item at a time.

However, I was able to come up with a recursive core.match version, though, if people prefer that. Only down side is it's 50% slower.

(defn ands->commas2
  "Like the above, but uses core.match recursively to look at two items at a time."
  [node]
  (match/match [node]
    [([n1 n2 & more] :seq)]
    (if (and (tree/whitespace? n1)
             (string? n2)
             (re-matches #"(?i)AND" n2))
      (cons "," (ands->commas2 more))
      (cons n1 (ands->commas2 (cons n2 more))))

    :else node))

I also tried using into [] and vectors, but lists are faster for building piece-wise.

(defn identifier-list->variable-list
[node]
(into [:variable-list]
(map (fif identifier->variable (tree/tag-pred :identifier)))
(rest node)))
(-> (into [:variable-list]
(map (fif identifier->variable (tree/tag-pred :identifier)))
(rest node))
ands->commas))

(defn identifier->density-event-eq
"Given a simple symbol node for returns the density event for when the
"Given an identifier node, returns the density event for when the
variable of that name equals the value held by that symbol in the
environment."
environment.

Returns the node unaltered if not an identifier."
[node]
(if-not (= :identifier (tree/tag node))
node
Expand All @@ -83,6 +109,7 @@
(defn strict-node
[node]
(match/match [(vec (remove tree/whitespace? node))]

Copy link
Contributor

Choose a reason for hiding this comment

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

Intentional? Fine either way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The blank line? Uhh, don't recall. But with I do tend to add blank lines when the body of something has pairs of lines, like long conditionals in cond.

[[:density-event-eq ([:identifier _] :as id) equals scalar-expr]]
[:density-event-eq (identifier->variable id) ws equals ws [:scalar-expr scalar-expr]]

Expand Down Expand Up @@ -140,6 +167,15 @@
with ws with-event ws
under ws model]

[[:given-except-clause except given-except-id-list]]
[:conditioned-by-except-clause except ws (identifier-list->variable-list given-except-id-list)]

[[:given-expr [:model-expr model] _given [:star star]]]
[:conditioned-by-expr [:model-expr model] ws "CONDITIONED" ws "BY" ws [:star star]]

[[:given-expr [:model-expr model] _given [:star star] except-clause]]
[:conditioned-by-expr [:model-expr model] ws "CONDITIONED" ws "BY" ws [:star star] ws except-clause]

[[:given-expr [:model-expr model] _given events]]
(transduce (map identifier->density-event-eq)
(completing model-expr)
Expand All @@ -161,7 +197,7 @@

(defn ->strict
[node]
(let [f (fn [x]
(let [f (fn strictify-branch-node [x]
(if (tree/branch? x)
(strict-node x)
x))]
Expand Down
10 changes: 8 additions & 2 deletions test/inferenceql/query/permissive/parser_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@
"model GIVEN x > 0 AND y = 0"
"model GIVEN x = 0, y = 0"
"model GIVEN x = 0, y > 0"
"model GIVEN x > 0, y = 0"))
"model GIVEN x > 0, y = 0"
"model GIVEN *"
"model GIVEN * EXCEPT (foo)"
"model GIVEN * EXCEPT foo, bar"
"model GIVEN * EXCEPT foo AND \"bar.none\" AND moop"))

(deftest given-invalid
(are [s] (insta/failure? (parser/parse s :start :given-expr))
"model GIVEN VAR x = 0 OR VAR y = 0"
"model GIVEN VAR x = 0 OR VAR y > 0"
"model GIVEN VAR x > 0 OR VAR y = 0"))
"model GIVEN VAR x > 0 OR VAR y = 0"
"model GIVEN * EXCEPT"
"model GIVEN * EXCEPTfoo, bar"))

(deftest generative-join-valid
(are [s] (not (insta/failure? (parser/parse s)))
Expand Down
18 changes: 18 additions & 0 deletions test/inferenceql/query/permissive_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,24 @@
"model GIVEN x AND y AND z"
"model CONDITIONED BY VAR x = x CONDITIONED BY VAR y = y CONDITIONED BY VAR z = z"))

(deftest given-star
(are [permissive strict] (= (strict.parser/parse strict :start :model-expr)
(-> permissive
(permissive.parser/parse :start :model-expr)
(permissive/->strict)))

"model GIVEN *"
"model CONDITIONED BY *"

"model GIVEN * EXCEPT (x, y)"
"model CONDITIONED BY * EXCEPT (VAR x, VAR y)"

"model GIVEN * EXCEPT x, y"
"model CONDITIONED BY * EXCEPT VAR x, VAR y"

"model GIVEN * EXCEPT x AND y AND z"
"model CONDITIONED BY * EXCEPT VAR x, VAR y, VAR z"))

(deftest probability
(are [permissive strict] (= (-> strict
(strict.parser/parse :start :scalar-expr))
Expand Down
Loading