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

copy_term/3 kind of removes constraint #1272

Closed
UWN opened this issue Feb 8, 2022 · 8 comments
Closed

copy_term/3 kind of removes constraint #1272

UWN opened this issue Feb 8, 2022 · 8 comments

Comments

@UWN
Copy link

UWN commented Feb 8, 2022

The idea of copy_term/3 is to leave the 'constraint store' completely unmodified. But this is what happens:

?- userpred.
caught: error(existence_error(procedure,userpred/0),userpred/0) % expected
?- freeze(X,userpred).
   freeze:freeze(X,user:userpred). % expected
?- freeze(X,userpred), copy_term(X,C,Rs).
   Rs = [freeze:freeze(C,user:userpred)]. % unexpected, missing above
?- freeze(X,userpred), copy_term(X,C,Rs), X = 1.
   X = 1, Rs = [freeze:freeze(C,user:userpred)]. % unexpected, missing below error
?- freeze(X,userpred), X = 1.
caught: error(existence_error(procedure,userpred/0),userpred/0) % expected
@triska
Copy link
Contributor

triska commented Feb 9, 2022

copy_term/3 should wrap the gathering of residual goals in findall/3 in order to undo any modifications that the gathering itself performs on attributes.

Conceptually, copy_term/3 should be written like this:

copy_term(Term, Copy, Gs) :-
        '$term_attributed_variables'(Term, Vs),
        findall(Term-Gs,
                ( phrase(gather_residual_goals(Vs), Gs),
                  '$delete_all_attributes'(Term)
                ),
                [Copy-Gs]).

We assume here that a nonterminal gather_residual_goals//1 is defined that invokes M:attribute_goals//1 for each variable in the argument with M being instantiated to the respective module that any attribute on this variable stems from.

The reason to use findall/3 is that M:attribute_goals//1 may destructively modify an attribute (or remove it) in order to simplify the projected goals, and to indicate that a residual goal was already generated for a particular constraint that may affect several variables yet needs to be stated only once, such as all_distinct/1.

Also, we assume the presence of an internal predicate called '$delete_all_attributes'/1 which deletes all attributes from any attributed variables in the given term. This is used to remove the constraints from the copy (or rather, from the term that is then copied, and which has its attributes reinstantiated by the backtracking that occurs due to findall/3).

@UWN
Copy link
Author

UWN commented Feb 12, 2022

For the moment, the defective copy_term(T,T,[]) can be replaced by \+ \+ copy_term(T,T,[]).

@triska
Copy link
Contributor

triska commented Feb 18, 2023

Regarding sort/2, please see this comment: 8a52ba0#r101125012

@UWN
Copy link
Author

UWN commented Feb 18, 2023

It seems to be less resource consuming to put '$term_attributed_variables'(Term, Vs), into the findall/3, this reclaims the space for Vs.

@UWN
Copy link
Author

UWN commented Feb 18, 2023

copy_term(Term, Copy, Gs) :-
   can_be(list, Gs),
   findall(Term-Rs, term_residual_goals(Term,Rs), [Copy-Gs]).

term_residual_goals(Term,Rs) :-
   '$term_attributed_variables'(Term, Vs),
   phrase(gather_residual_goals(Vs), Rs),
   '$delete_all_attributes'(Term).    % isn't Vs better?

@triska
Copy link
Contributor

triska commented Feb 18, 2023

'$delete_all_attributes'(Term). % isn't Vs better?

Yes, and on a general note, I would like to add that it is highly desirable to keep as much code as possible in Prolog, because only then will there be the greatest motivation to actually improve the performance of the Prolog code. Putting it all in Rust is tempting, but should be seen as a last resort: The key design point is finding exactly the smallest set of primitives that absolutely must be placed in Rust for acceptable performance, or to enable bootstrapping.

In this concrete case, it seems preferable to implement the internal Rust-based '$delete_all_attributes'/1 so that it works for a single variable, and then use it for all Vs that were gathered by a previously used '$term_attributed_variables'/1.

@UWN
Copy link
Author

UWN commented Feb 19, 2023

What about the deeper question, whether or not findall/3 should copy attributes at all. So far, SICStus never copies constraints and for those who need it, copy_term/3 was provided. That was the very purpose of it.

?- X in 1..2.
   clpz:(X in 1..2).
?- findall(X,X in 1..2,Xs).
   Xs = [_A], clpz:(_A in 1..2).
?- X in 1..2, copy_term(X,Y).
   clpz:(X in 1..2), clpz:(Y in 1..2).
?- X in 1..2, asserta(fact(X)), fact(Y).
   clpz:(X in 1..2).
?- setof(X,(X in 1..2 ; X in 2..3), Xs).
   Xs = [_A,_B], clpz:(_A in 1..2), clpz:(_B in 2..3).
?- setof(X,(X in 1..3, (X in 1..2 ; X in 2..3)), Xs).
   Xs = [_A,_B], clpz:(_A in 1..2), clpz:(_B in 2..3).
?- setof(X,(X in 1..2 ; X in 1..2), Xs).
   Xs = [_A,_B], clpz:(_A in 1..2), clpz:(_B in 1..2).
?- catch((X in 1..2, throw(ex(X))), Pat, true).
   Pat = ex(_A), clpz:(_A in 1..2).

@triska
Copy link
Contributor

triska commented Feb 19, 2023

I think findall/3 is worth discussing in a separate issue? In this way, we can focus on copy_term/3 here, does it now work as expected and can we close this issue and the related issue #923?

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

2 participants