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

tryBinarySearch completely failed on [(0x674290, 0x62a180)] and will now backtrack to fix an upstream problem. #156

Closed
sei-eschwartz opened this issue Jan 22, 2021 · 12 comments

Comments

@sei-eschwartz
Copy link
Collaborator

Feels rather hacky to modify the pharos files.
Anyway with and without the purecall(0x634d60). hack I now get

tryBinarySearch completely failed on [(0x674290, 0x62a180)] and will now backtrack to fix an upstream problem.
Refusing to backtrack into reasoningLoop to fix an upstream problem because backtrackForUpstream/0 is not set.
This likely indicates that there is a problem with the OO rules.

with the latest docker image.

Originally posted by @Trass3r in #139 (comment)

@sei-eschwartz sei-eschwartz self-assigned this Jan 22, 2021
@sei-eschwartz
Copy link
Collaborator Author

This needs to be triaged to see if it's related to shared implementations or something else.

@Trass3r
Copy link
Contributor

Trass3r commented Jan 22, 2021

With trivial(0x634d60). in initial.pl I get the same error, just different values: tryBinarySearch completely failed on [(0x674b18, 0x61edf0)]

@sei-eschwartz
Copy link
Collaborator Author

I am rerunning this now to see if we get the same problem on our internal branch.

@sei-eschwartz
Copy link
Collaborator Author

Slightly different error, but probably the same issue:

tryBinarySearch completely failed on [(0x674290, 0x6197b0)] and will now backtrack to fix an upstream problem.

We will investigate soon.

If you have a chance, it would help if you could do some analysis and offer your opinion on some of the related conclusions:

  • Is 0x61ed60 a constructor?
  • Is 0x674290 on the same class as 0x6197b0?

@Trass3r
Copy link
Contributor

Trass3r commented Jan 29, 2021

Yeah looks like a constructor. Which means that 61ECA0/61ED10 is yet another custom new/delete.
0x6197b0 is reused in a lot of classes, 674290 is one of them.

@sei-eschwartz
Copy link
Collaborator Author

Hmm. Definitely looks like 0x6197b0 may be a shared implementation.

@Trass3r
Copy link
Contributor

Trass3r commented Jan 29, 2021

Unless those classes are related and the method implementation is inherited.

@sei-eschwartz
Copy link
Collaborator Author

Yeah, I could see it going either way. It's a very simple method. But most of the problems that we've had with shared implementations were not guesses, so this probably isn't the underlying problem...

@sei-eschwartz
Copy link
Collaborator Author

I changed OOAnalyzer to output the sanity check that fails even when the result comes from tabling. This is the result after the initial guess fails:

The guess tryMergeClasses(0x674290, 0x62a180) was inconsistent with a valid solution.
Guessing tryNOTMergeClasses(0x674290, 0x62a180) instead.
Guessing factNOTMergeClasses(0x674290, 0x62a180).
There are 96,797 known facts.
reasoningLoop: pre-reason sanityChecks
failed.
Consistency checks failed.
Contradictory information about constructor: factConstructor(0x61ed60) but reasonNOTConstructor(0x61ed60)
Constraint checks failed, retracting guess!
Fail-Retracting guessedNOTMergeClasses(0x674290, 0x62a180)...
Fail-Retracting factNOTMergeClasses(0x674290, 0x62a180)...

This is suspicious because reasonNOTConstructor(0x61ed60) became true during the guess of factMergeClasses(0x674290, 0x62a180) and it was not reinferred when the guess fails. I think this may be a tabling bug in SWI, rather than an issue with our rules. I will investigate more later this weekend.

@sei-eschwartz
Copy link
Collaborator Author

There is definitely some kind of problem with incremental tabling. I am working on a bug report for the SWI maintainer.

@sei-eschwartz
Copy link
Collaborator Author

@JanWielemaker fixed a bug in SWI (thanks Jan!) and I was now able to run this through using the master branch of SWI prolog.

Here's the resulting json if you want to take a look:
DKII.EXE.json.zip

@sei-mwd
Copy link
Collaborator

sei-mwd commented Feb 11, 2021

Commit 2888c62 revises commit 0d15dc4 to warn about older versions of SWI Prolog instead of requiring the most recent version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants