-
Notifications
You must be signed in to change notification settings - Fork 34
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
Satisfying assignment found when using SArray
, not with SFunArray
#541
Comments
|
I pushed in a fix that should address the issue. When I run your program, I now get:
Note that this is a different output from what you get with an |
Well the problem with reducing the repro code is that I have absolutely no idea what the output means anymore, I need to go back to the full program for that. |
In the abstract, this doesn't sound OK: just like the Hex example you have pointed me towards, my code keeps feeding more and more input to the stateful computation only if the input so far doesn't yield a satisfying end state. So if |
OK so going back to my original program, I get two different solutions for I tried to "prove" this, and the best I could come up with was to change
If Would it be helpful if I ported this "checker" to the minimized version as well? |
I believe the But you've the right idea: Whatever solution you get from whichever array representation, it'd be good to add code that walks through it concretely and makes sure it indeed satisfies all your constraints. If you find otherwise, that'd be a bug. (Either in the end-user code or in SBV.) Do you still get a failure when you change Let me know what you find out! |
Yeah but that's because I went back to the original program, not the cut-down one, and that one gave me |
OK so here's a full standalone program that demonstrates the problem. In line 21, I define the type synonym
Output with
Output with
And from the "outside POV", the proposed solution |
Thanks for the report. Looking at this in some detail; I'm noticing that the caching mechanism used by I'm not sure if this is something I can work around easily. I'm inclined to put in the code to check if |
Thinking more about this, I'm no longer sure if it's Is there any chance you can simplify it? Say, get rid of all the state-constructions, and mergeable instances? That'd make it simpler to continue debugging this issue. |
Actually, I just noticed that the program you posted in this ticket behaves differently on my machine. Regardless
I'm curious why you're getting different output. Can you download SBV from github master and try again, maybe you're referring to a stale version somehow? |
I went through various commits trying to reproduce this:
|
But I thought 375ae01 was supposed to just be a revert of e3f4f8a? Looking at the diff between 375ae01 and e3f4f8a^, the combined effect of the two commits is only in
|
Yes, I'm a bit confused as well. The above change was bogus, which I now reverted. Bottom line, the latest master in github is the one to put to test. |
Well like I said, with 375ae01 which I see as the latest, it works. |
Right.. Does it work for your full repo as well? |
OK just checked, and... no :/ |
That's what I expected.. If you can create a new repro, I'd like to take a look. It might indeed end up being a bug in SBV, but so far I'm not seeing why. I'd like to get to the bottom of it. |
While I am working on minimizing it, here's the full code for reference: https://github.com/gergoerdi/scottcheck/tree/sbv-issue-541 You can run it with |
Thanks. Would be really great to get rid of all those Mergeable's, lenses, and anything higher-order, if you can. The simpler, the better. (When we get to the bottom of this, I'm sure it'll be demonstrable with a 10 line program.) |
You can track my progress in simplifying it at that branch:) |
Waiting eagerly to see with what you come! Thanks for helping debug this. |
OK so I managed to remove all uses of Lens, MTL, and Transformers. Let me know if it's good enough for debugging now. |
Great.. The simpler the better, but I'll take a look at this version. |
I've found something that could potentially be interesting: a simplification step that makes the On top of fcead9ea91e86e34c14e357e79804e75c3ce584e, if I just change the definition of
to
(which means we will not need to take the 18 branch of So maybe you could look at the generated constraints with and without this one-liner change. |
Here's the smallest version I have so far: <50 lines, single module:
|
We can even get rid of
|
I think this is the smallest I can do myself:
|
Fantastic! Great progress, and most importantly there are no import Data.SBV
test a = sat $ do
arr <- newArray "items" (Just 0)
x <- sInteger "x"
let ys = writeArray (arr `asTypeOf` a) 0 255
idx = ite (x .== 1) 0 1
y = readArray ys idx
ys1 = writeArray ys idx $ ite (y .== 1) 255 y
ys2 = writeArray ys idx $ ite (y .== 255) 1 y
ys' = ite (x .== 0) ys1 $ ite (x .== 1) ys2 ys
constrain $ readArray ys' 0 .== 1
main = do print =<< test (undefined :: SArray Integer Integer)
print =<< test (undefined :: SFunArray Integer Integer) I get:
This is clearly a bug! And hopefully small enough to figure out what's going on. |
Update: I've spent some time on this, and I'm still stumped on what's going on. Even though the failing program is rather small now, it's really intricate how the Jeff: If you find yourself with free time and want to lend a pair of fresh eyes, do take a look. The functions to worry about are |
@LeventErkok Will do but I won't have any time until later this week unfortunately. |
Here's another program that produces a bad result, I believe it's caused by the very same issue: import Data.SBV
test :: Bool -> IO SatResult
test swap = satWith z3{verbose=False} $ do
arr :: SFunArray Integer Integer <- newArray "items" (Just 0)
x <- sInteger "x"
let ys = writeArray arr 0 2
idx = x + 1
ys1 = writeArray ys 0 (readArray ys idx)
ys2 = writeArray ys idx 1
v = if swap
then ite (x .== 0) (readArray ys1 0) (readArray ys2 0)
else ite (x ./= 0) (readArray ys2 0) (readArray ys1 0)
constrain $ v .== 1
run :: IO ()
run = do print =<< test False
putStrLn $ replicate 40 '-'
print =<< test True When I run it, I get:
Note that the only difference is the swapping of the Obviously, caching isn't working correctly here: It appears to produce different results depending on evaluation order. The caches for the array will surely be different depending on the order of evaluation, but I'm not sure why the latter is producing an |
…rther accesses can use this entry symbolically Addresses #541
I just pushed a few commits that I believe should fix this issue. Can you give it a try and see if it works for your original program? |
Thanks, I can confirm it works now on my full program: using |
Great! Out of curiosity: What's the performance of |
OK so I finally got around to testing this, with 34d836a. And the results are... inconclusive. I have two input files, a smaller one and a larger one; the smaller one is a simpler problem that requires less stateful steps to solve. For the small one,
|
Thanks for the report. I've observed similar over the years, where it's really hard to predict how the array-solvers in SMT-land will behave. In theory, |
The code at https://github.com/gergoerdi/scottcheck/tree/sbv-sfunarray-bug uses an
SFunArray
insrc/ScottCheck/Engine.hs
, line 59. The program can be run withstack run
without any external dependencies. With thatSFunArray
version, the search gets to depth 8 and then starts slowing down.However, if I replace that
SFunArray
withSArray
, the search correctly finds a solution at just depth 6:The text was updated successfully, but these errors were encountered: