ridgeworks / clpBNR

CLP(BNR) module for SWI-Prolog
MIT License
38 stars 5 forks source link

"Output" predicate #4

Closed JeanChristopheRohner closed 3 years ago

JeanChristopheRohner commented 3 years ago

A suggestion is to have some kind of output predicate that always has the same structure, no matter what the solution is.

In the current version, for example, if i do ?- {X==1/2}, X::DOM. i get false. But if i do ?-{X*X==2} i get DOM = real(-1.4142135623730951, 1.4142135623730951). With these numbers it is easy to know what happens beforehand, but in other cases it might be unclear what unifies with X.

Something like output(+X, -Variable, -Lower, -Upper, +PreferredNumberOfDecimals) which given ?-{X == 1/2}, output(X, Variable, Lower, Upper, 3) would yield Variable = _32424, Lower = 0.500, Upper = 0.500.

Just a humble suggestion :-)

ridgeworks commented 3 years ago

My current belief is that output formatting is likely to be largely application dependent so I've tried to steer away from making any decisions for the end user in this regard. In general output can be quite complicated as demonstrated by the numerous options of format/1,2,3 (see https://www.swi-prolog.org/pldoc/man?section=format) and term writing (see https://www.swi-prolog.org/pldoc/man?section=termrw).

So what I tried to do was make sure all of the SWIP term writing support would work with intervals (clpBNR attributed variables). When the SWIP environment flag write_attributes is set to portray (automatically done when consulting clpBNR), the attribute_portray_hook will map the clpBNR attribute to its domain so:

?- X::real,write(X).
_139560{real(-1.0e+16,1.0e+16)}
X::real(-1.0e+16, 1.0e+16).

Note the output format - this is SWIP standard for CLP, e.g.,

?- use_module(library(clpfd)).
true.

?- X#<0, write(X).
_159010{clpfd = ...}
X in inf.. -1.

Now clpfd doesn't do much with its attribute, but that's a separate issue.

The other "opportunity" for formatting output is what's done for the top level Prolog "listener". Here the SWIP convention is to output attributed variables as a list of goals which when executed re-creates the attributed variable, as shown in the examples above.

For historical reasons, the predicate print_interval/1,2 is also provided. Now this is where I have more flexibility, but options are limited. For now it just prints out the domain of any interval in the term so it's effectively the same (I think) as your early experiments with pretty printing:

?- X::real,print_interval(f(X,Y)).
f(real(-1.0e+16,1.0e+16),_215804)
X::real(-1.0e+16, 1.0e+16).

So I'm open to suggestions on what print_interval should do but I don't want to slide down the slippery slope of providing highly flexible output specific to intervals; that's a job better suited to applications developers.

Getting back to your specific example, using {X==1/2} is problematical because this doesn't result in X being an interval at all. 1/2 is parsed by SWIP as the rational constant 1r2 which is precise and results in an interval point value which is then unified with X. If you had used a float (all floats are considered imprecise) than an interval would result:

?- {X1==1/2.0, X2==1/2}.
X2 = 1r2,
X1:: 0.500000000000000... .

X1 is an interval while X2 is not.

Bottom line:

  1. I don't want to add a lot of output options because I think that's a job better left to the user who can exploit the rich set of options already available in SWIP. If there's some mechanism which can be provided in clpBNR to make this job easier, that's worth considering.
  2. I'm happy to consider changes to print_interval within the envelope of the current predicate defintion, i.e., no changes in arguments, if that's useful.

P.S. In one of your earlier examples you use term_string twice to effectively make a copy of a term. You should look at copy_term and copy_term_nat as (IMO) a better way of doing this.

JeanChristopheRohner commented 3 years ago

Thanks for taking the time to write such a thorough response.

"I don't want to add a lot of output options because I think that's a job better left to the user who can exploit the rich set of options already available in SWIP. If there's some mechanism which can be provided in clpBNR to make this job easier, that's worth considering."

Now that i understand this better i agree.

"I'm happy to consider changes to print_interval within the envelope of the current predicate defintion, i.e., no changes in arguments, if that's useful."

It makes sense to not change the predicate definition for print_interval/1,2. My main motivation was that I was trying to get write/1 and co to print something that looks as nice as what one gets when using the top level. I.e. instead of:

?- X::real,write(X).
_22164{real(-1.0e+16,1.0e+16)}
X::real(-1.0e+16, 1.0e+16).

My wish was to have:

?- X::real,myWrite(X).
A::{real(-1.0e+16,1.0e+16)}
X::real(-1.0e+16, 1.0e+16).

where A comes from numbervars/2,3.

"P.S. In one of your earlier examples you use term_string twice to effectively make a copy of a term. You should look at copy_term and copy_term_nat as (IMO) a better way of doing this."

I see :-) Thanks for the suggestion.

And thanks again for porting clpBNR. I have been trying it out the last could of days; I am really impressed with its ability to deal with non-linear equations, maximise and minimise (and even find partial derivatives).

ridgeworks commented 3 years ago

Based on your input, I think I will revisit print_interval to make it more useful. It probably wouldn't take much to add alphabetic var "names" with their domain, e.g., A::real(-1.0e+16, 1.0e+16). It may even be possible to optionally (using clpBNR_verbose flag?) capture the whole constraint network as (persistent) text, although I'm not sure exactly how that would be used.

Thanks for taking the time to beat on this. I think it's getting to the point where some user feedback is very helpful. There should be a new version (0.9.2) available in the next week or two - mostly small optimizations, bug fixes, and some additional User Guide material.

ridgeworks commented 3 years ago

Suggested enhancement to print_interval:

?- X::real(0,1), print_interval([f(X,Y,[X])]), nl, write([f(X,Y,[X])]).
[f(A::real(0,1),B,[A::real(0,1)])]
[f(_27364{real(0,1)},_26422,[_27364{real(0,1)}])]
X::real(0, 1).

So more or less the same as top level except the ellipsis output style isn't supported. I omitted that because it's not valid input syntax and it loses information (trailing digits).

Better?

JeanChristopheRohner commented 3 years ago

That would be great to have. Thanks for looking into this.

I am not sure howprint_interval(+X, -STREAM) works, but it would be great if STREAM is something like interval(VARNAME, real(-1, 1)) so that it is possible to choose what names are assigned to VARNAME with numbervars/3 (e.g. A, B, ..., Z; or P, Q, R; etc).

ridgeworks commented 3 years ago

print_interval(Stream,Term) works the same way as write/2, i.e., it allows you to redirect output, e.g., to a file. So it doesn't affect the output variable names. However, the new print_interval implementation is pretty simple and doesn't use any internal predicates:

%
%  print_interval(Term), print_interval(Stream,Term)
%   prints Term to optional Stream with intervals expanded to domains
%   uses format/3 so extended Stream options, e.g., atom(A), are supported
%
print_interval(Term) :- print_interval(current_output,Term).

print_interval(Stream,Term) :-
    copy_term_nat(Term,Out),     % copy of Term without attributes for output
    term_variables(Term,TVars),
    term_variables(Out,OVars),
    map_attr_vars(TVars,OVars),  % convert corresponding interval variables to decls
    numbervars(Out,0,_),         % number vars on Out
    format(Stream,'~w',[Out]).

map_attr_vars([],[]).
map_attr_vars([Int|TVars],[V::Dom|OVars]) :-
    domain(Int,Dom), !,
    map_attr_vars(TVars,OVars).
map_attr_vars([TVar|TVars],[OVar|OVars]) :-
    map_attr_vars(TVars,OVars).

so it's easy to clone it and add your own embellishments.

JeanChristopheRohner commented 3 years ago

Great. Thanks again!