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

Prefix to infix notation #19

Open
neighthan opened this issue Sep 29, 2022 · 0 comments · May be fixed by #21
Open

Prefix to infix notation #19

neighthan opened this issue Sep 29, 2022 · 0 comments · May be fixed by #21

Comments

@neighthan
Copy link

I'm not sure if this is a bug, so I just wanted to check: how should the prefix notation for arithmetic operations (+, -, *, /) be converted to infix notation? I expected that (- x y) was the same as (x - y), but it seems to be (y - x) instead. However, for division, it seems that (/ x y) does mean (x / y), not (y / x). To me, this seems confusing and likely to lead to errors. Is this the intended behavior or a bug?

I tested this using the following domain and problem files:

Domain

(define (domain debug)

(:requirements :numeric-fluents)

(:functions
  (x)
)

(:action xInc
  :parameters ()
  :precondition (and )
  :effect (and
    (assign (x) (+ x 1))
  )
)

(:action xDec
  :parameters ()
  :precondition (and )
  :effect (and
    (assign (x) (- x 1))
  )
)

(:action xMul
  :parameters ()
  :precondition (and )
  :effect (and
    (assign (x) (* x 1))
  )
)

(:action xDiv
  :parameters ()
  :precondition (and )
  :effect (and
    (assign (x) (/ x 1))
  )
)

)

Problem

(define (problem debug1) (:domain debug)

(:init
  (= x 0)
)

(:goal (and
  (= x 10)
))

)

Then I run smtplan with the -n option to print the constraints out. After parsing with z3 (so they're easier to read), the relevant constraints are

Implies((xinc)0_sta, (x)0_1 == 1 + (x)0_0),
Implies((xdec)0_sta, (x)0_1 == 1 - (x)0_0),
Implies((xmul)0_sta, (x)0_1 == 1*(x)0_0),
Implies((xdiv)0_sta, (x)0_1 == (x)0_0/1),
neighthan added a commit to neighthan/SMTPlan that referenced this issue Feb 15, 2023
Fixes KCL-Planning#19.

`visit_div_expression` already had the correct method of popping the RHS first then the LHS. However, plus, minus and mul all switched the order and called the first thing popped from the stack the LHS. For plus and mul, this doesn't affect the correctness, but it lead to minus being interpreted backwards (see the linked issue). I think this should fix that issue, but I'm not familiar with the code base, so please let me know if there are other places this change would need to be made too.
@neighthan neighthan linked a pull request Feb 15, 2023 that will close this issue
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 a pull request may close this issue.

1 participant