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

The code fails to run due to the discontiguous predicate definition lines #3

Open
gabouelseoud opened this issue Aug 4, 2021 · 26 comments

Comments

@gabouelseoud
Copy link

I describe the problem in this thread and the colleague who replied also failed to get the expected output.
mthom/scryer-prolog#1014 (comment)
I hope you can help us

@flexoron
Copy link

flexoron commented Aug 4, 2021

At the home-page there are 3 examples where you can try and go:
https://www.metalevel.at/prolog/timetabling/

(Cut/Paste) Example 1 and 2 do work(but no timetable.txt has been written)

$ ../scryer-prolog/target/release/scryer-prolog simsttab.pl example1.pl OK!
$ ../scryer-prolog/target/release/scryer-prolog simsttab.pl example2.pl OK!

Example 3 failed:
$ ../scryer-prolog/target/release/scryer-prolog simsttab.pl example3.pl
?- requirements_variables(Rs, Vs),labeling([ff], Vs),print_classes(Rs).
false.
?-

@gabouelseoud
Copy link
Author

@flexoron Thank you very much. Until triska replies, you provided me with a very good starting point. I'll try to better understand the code to find out what the problem is. I really appreciate your precious support.

@triska
Copy link
Owner

triska commented Aug 4, 2021

I can reproduce this! There is definitely a mistake somewhere, the only question is where exactly!

Let's break it down. First, we see that the first goal by itself already fails:

?- requirements_variables(Rs, Vs).
false.

So, adding more goals (i.e., constraints), will fail "all the more".

One thing I noticed is that we "generalize away" the last goal in requirements_variables/2, then it succeeds. To generalize away a goal, we use (*)/1 from library(debug). This is similar to "commenting out" a goal, with the advantage that it also works for the last goal in a clause, which is followed by .. So, if we change the definition of requirements_variables/2 to:

requirements_variables(Rs, Vars) :-
        requirements(Rs),
        pairs_slots(Rs, Vars),
        slots_per_week(SPW),
        Max #= SPW - 1,
        Vars ins 0..Max,
        maplist(constrain_subject, Rs),
        classes(Classes),
        teachers(Teachers),
        rooms(Rooms),
        maplist(constrain_teacher(Rs), Teachers),
        maplist(constrain_class(Rs), Classes),
        * maplist(constrain_room(Rs), Rooms).

then it succeeds.

This all worked as expected in earlier versions of Scryer Prolog. Are you interested in trying git bisect on the Scryer repository to pinpoint the commit that caused this problem? Please try it with a version of Scryer Prolog from March 2020, since the latest commit to simsttab.pl is from that time and I am certain it worked back then. For instance, you could start at 54dce9b60eff2b2c2f940554f4f9952986e6f8c6 and then bisect your way to more recent versions!

@flexoron
Copy link

flexoron commented Aug 4, 2021

git bisect drives compiling into crates hell

For example:
Compiling lexical-core-0.4.6
error[E0308]: mismatched types
lexical-core-0.4.6/src/atof/algorithm/bigcomp.rs:243:55
let nlz = den.leading_zeros().wrapping_sub(wlz) & (u32::BITS - 1); -> expected usize, found u32

After fixing those and other errors in bigcomp.rs, bigint.rs and math.rs
Next error:

Compiling markup5ever v0.8.1
error[E0603]: module export is private
markup5ever-0.8.1/build.rs:92:14
#[derive(Deserialize, Debug)] --> private module error (due to serde-1.0.127)
Cargo.toml dependency entry fixes this : serde = "1.0.123"

Finally it compiles but now I get this syntax error message, too
$ ../scryer-prolog/target/release/scryer-prolog simsttab.pl reqs.pl
caught: error(syntax_error(inconsistent_entry),use_module/1)

hmm...I'm not an rust expert but it looks like you need the rust environment of March 2020 as well when you reset to 54dce9b60eff2b2c2f940554f4f9952986e6f8c6

@triska
Copy link
Owner

triska commented Aug 4, 2021

Thank you a lot for looking into this!

At some point, Scryer did not yet support the discontiguous directive, this is a rather recent feature!

To prevent this syntax error, you can comment out the discontiguous directives in simsttab.pl, and then you must also ensure that all clauses in reqs.pl are actually contiguous. You can do this in Emacs by marking the hole buffer (C-x h) and then sorting all facts alphabetically with M-x sort-lines RET.

@gabouelseoud
Copy link
Author

gabouelseoud commented Aug 4, 2021 via email

@flexoron
Copy link

flexoron commented Aug 4, 2021

this works, too:
reqs.pl
%room_alloc(r1, '1a', sjk, 0).
%room_alloc(r1, '1a', sjk, 1).
room_alloc(r1, '4c', mat, 0).
room_alloc(r1, '4d', mat, 0).

the issue is with:
room_alloc(r1, '1a', sjk, 0).
room_alloc(r1, '1a', sjk, 1).

@flexoron
Copy link

flexoron commented Aug 4, 2021

this works, too:
reqs.pl
room_alloc(r1, '2a', sjk, 0).
room_alloc(r1, '2a', sjk, 1).
room_alloc(r1, '4c', mat, 0).
room_alloc(r1, '4a', mat, 0).

the system does not like (to be:-) '1a'

@gabouelseoud
Copy link
Author

gabouelseoud commented Aug 5, 2021 via email

@gabouelseoud
Copy link
Author

I just tried modifying the reqs file as triska described, so that lines referring to the same predicate are together and I commented out the discontinguous (files are attached) and I run using an older version of scryer that I installed using the command: cargo install scryer-prolog (instead of cargo run --release). It now works. However, I am worried about not understanding why the new version fails. There is a bug in the new engine that makes it fail to find a solution where it exists, and unless we know what it is, we may be getting wrong results. (I changed from .pl to .txt to be able to attach the files here for you to review them)
simsttab_old.txt
reqs_new.txt

@flexoron
Copy link

flexoron commented Aug 5, 2021

@triska: sorting is not an option because when sorted both scryer versions do the job.
I wonder how Example3 (https://www.metalevel.at/prolog/timetabling) works(it is not sorted)?

@gabouelseoud
Copy link
Author

gabouelseoud commented Aug 5, 2021

example 3 doesnot work with the new version and old version both.
Sorting works for me only with the old version after commenting out discontinguous lines as in attached files in my previous message,
NO, sorting doesn't work with the newer version, I get: Segmentation fault (core dumped)
and before that huge memory consumption that ends in fault. (I am assuming you mean by sorting the modification I did for the reqs.pl file in my version reqs_new.pl in my previous message)

@flexoron
Copy link

flexoron commented Aug 5, 2021

example3.pl is cut/paste from www.metalevel.at/prolog/timetabling

Well, here it works if sorted with this command.

$ sort example3.pl > example3-sorted.pl

$ # discontinguous disabled
$ ../scryer-prolog-old/target/release/scryer-prolog simsttab-old.pl example3-sorted.pl

$ # discontinguous enabled
$ ../scryer-prolog-new/target/release/scryer-prolog simsttab-new.pl example3-sorted.pl

@gabouelseoud
Copy link
Author

@flexoron No, I tried your version example3-sorted.pl and got exactly my same earlier results:
old version works smoothly
new version of scryer consumes a lot of memory and ends in segmentation fault.
It appears that the newer version seeks a more (un-necessarily) memory-consuming route to solution. May be your computer has better memory resources than mine ;that's why you don't get segmentation fault. But at any event, this seems to be a bug since the old scryer finds the solution quite smoothly. I have 4 G ram

@flexoron
Copy link

flexoron commented Aug 5, 2021

yes, the new release version(without debug info) consumes twice as much memory.
old version: 1.5G
new version: 3.0G

@flexoron
Copy link

flexoron commented Aug 5, 2021

segfault: this is a scryer issue.
Rust is designed to avoid segmentation faults because segmentation faults are programmer's fault(usually).
Maybe triska gives a hint how to tell scryer's rust programmers about the issue.
Maybe it is known already.
Maybe it is caused intentionally.

@triska
Copy link
Owner

triska commented Aug 5, 2021

Yes, please report a segmentation fault in the Scryer Prolog repository, with a clear test case that exhibits the problem. No matter what else happens, Scryer definitely should not crash, so a crash is certainly a mistake in the implementation. If memory runs out, the system should throw an exception (resource_error) instead of crashing.

In addition, if everything works as expected if the facts are sorted, but then no longer works if the facts are in a different order, then this may be a mistake in the way the discontiguous directive is implemented. If we can construct a test case that exhibits this issue, we can likewise report it in the Scryer repository, in its own dedicated issue.

@gabouelseoud
Copy link
Author

gabouelseoud commented Aug 6, 2021 via email

@flexoron
Copy link

flexoron commented Aug 6, 2021

When directives are defined in reqs.pl, it works. (and so does Example 3, unsorted).

$ cat reqs.pl

% Simplified: 1 teacher, 1 subject, 2 classes, 2 x 5 lessons, 2 free slots for both classes at Mon, 1 day off at Tue.
% Hint: the configuration demands coupling for both classes due to 1 day off.

:- discontiguous(class_subject_teacher_times/4).
:- discontiguous(coupling/4).
:- discontiguous(class_freeslot/2).
:- discontiguous(room_alloc/4).

slots_per_week(20).
slots_per_day(4).

teacher_freeday(sjk1, 1).

class_subject_teacher_times('1a', sjk, sjk1, 5).
coupling('1a', sjk, 0, 1).
class_freeslot('1a', 0).
class_freeslot('1a', 1).
room_alloc(r1, '1a', sjk, 4).

class_subject_teacher_times('1b', sjk, sjk1, 5).
coupling('1b', sjk, 0, 1).
% shift coupling, try one of those:
% coupling('1b', sjk, 1, 2).
% coupling('1b', sjk, 2, 3).
% coupling('1b', sjk, 3, 4).
class_freeslot('1b', 2).
class_freeslot('1b', 3).
room_alloc(r1, '1b', sjk, 4).

@gabouelseoud
Copy link
Author

gabouelseoud commented Aug 6, 2021 via email

@flexoron
Copy link

flexoron commented Aug 7, 2021

This application is new to me. ok?

Here on my system, scyrer stabilizes its memory after a while and stops consuming.
The CPU temperature shows slow growth..

Anyway, my kiddy(1b) likes coupling requirement
@triska
coupling(C, S, L1, L2)
For class C, lessons L1 and L2 of subject S are to be scheduled in direct succession.

If direct succession here means the last slot of a day and the first slot of the next day are coupled
then the requirement is fulfilled.
If coupling should be 2 slots of a day sequenced(a block)
then simsttab.pl needs refinement.

With reqs.pl above: censor '1b', demand second coupling.
coupling('1b', sjk, 0, 1).
coupling('1b', sjk, 3, 4).

block(3,4) splits at times.

@triska
Copy link
Owner

triska commented Aug 7, 2021

Yes, you are right, well spotted!

Currently, coupling constraints are erroneously satisfied even if the coupling crosses a day boundary. This is obviously not what is intended with a coupling.

Let's please discuss this and possible solutions in a separate issue or pull request. Thank you a lot! Update: This is addressed via ad67f87.

@triska
Copy link
Owner

triska commented Aug 7, 2021

Regarding separation of issues, I have a general comment: Please, in general, let's keep an issue to a single topic. We are currently facing 3 different issues:

  • correctness of the discontiguous directive. This may be a true correctness issue with Scryer Prolog, and I think should be treated with the greatest priority. For this, a clear test case should be constructed so that Mark can easily reproduce the problem. A test case should give the exact invocation of Scryer Prolog and all necessary data to Mark, so that he can simply copy&paste the instructions and compare the results with various Scryer Prolog versions. We now have problems with new release version and simsttab time tabling software mthom/scryer-prolog#1015 which contains the necessary information, but is not easily understandable and reproducible for other interested contributors. If possible, please add specific command line invocations that show exactly how to reproduce the problem, what the expected output is, and what happens instead.
  • performance of Scryer Prolog. I agree that higher performance is better. However, higher performance of a specific instance does not help us if the solution is not even correct, so I would first focus on clearly explaining the correctness issue, so that Mark can look into that. After this works as expected, and if it is then still slower, we can look into the performance. A crash, on the other hand, should never arise, and should be filed as a separate Scryer Prolog issue immediately.
  • semantics of coupling constraints. This is the latest issue we are now also discussing. This is specific to simsttab and would be good to discuss in a separate issue in this repository. Update: Addressed via ad67f87.

Thank you a lot for looking into all these issues, these will improve Scryer Prolog and also simsttab!

@flexoron
Copy link

flexoron commented Aug 7, 2021

That's all well and good, but we need your help because it is your application which does not work as expected.

Where does the expectation come from? From simsttab's homepage, right?

So, please: Why does Example 3 work there (try it! ... go)

Semantics of coupling constraints: Yes this is a side-effect issue and is simsttab specific.
Performance of Scryer Prolog: Example 3 answers quick. So it seems it runs on a powerhouse.
Correctness of directives: Example 3 is unsorted so discontiguous directive seems to be declared somewhere?
Where did you place the directives? (and if directives are not defined, why does it work there and not elsewhere?)

@triska
Copy link
Owner

triska commented Jan 12, 2022

With the rebis-dev branch of Scryer Prolog, this mostly works, except for one remaining issue: mthom/scryer-prolog#1202

As a workaround until this issue is resolved, you can add the following directives at the start of example3.pl:

:- discontiguous(class_subject_teacher_times/4).
:- discontiguous(class_freeslot/2).
:- discontiguous(coupling/4).

The rebis-dev branch also entails significant performance improvements compared to master, I hope it works well for you!

@gabouelseoud
Copy link
Author

gabouelseoud commented Jan 14, 2022 via email

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

No branches or pull requests

3 participants