Incorrect behavior for "empty" adjective applied to 1-to-1 relations

In Relations.i6t there is a routine RelationEmptyOtoO(). Its function is to either test whether a 1-to-1 relation is empty (i.e. return true if no assertions exist and false if at least one exists) or, if the clear flag is set, to negate all assertions. Here is the routine as seen in 6M62 with additional comments to label the four included loops:

unmodified version
[ Relation_EmptyOtoO relation sym clear relation_property obj1 obj2 t1 t2 N1 N2;
    relation_property = RlnGetF(relation, RR_STORAGE);
    t1 = KindBaseTerm(RlnGetF(relation, RR_KIND), 0); ! Kind of left term
    t2 = KindBaseTerm(RlnGetF(relation, RR_KIND), 1); ! Kind of right term
    if (t2 == OBJECT_TY) {
        objectloop (obj2 provides relation_property) {			! loop 1
            obj1 = obj2.relation_property;
            if (obj1) {
                if (clear) obj2.relation_property = nothing;
                else rfalse;
            }
        }
    } else {
        for (obj2=1: obj2<=N2: obj2++) {						! loop 2
            obj1 = GProperty(t2, obj2, relation_property);
            if (obj1) {
                if (clear) WriteGProperty(t2, obj2, relation_property, 0);
                else rfalse;
            }
        }
    }
    if (t1 ~= t2) {
        if (t1 == OBJECT_TY) {
            objectloop (obj1 provides relation_property) {		! loop 3
                obj2 = obj1.relation_property;
                if (obj2) {
                    if (clear) obj1.relation_property = nothing;
                    else rfalse;
                }
            }
        } else {
            for (obj1=1: obj1<=N2: obj1++) {					! loop 4
                obj2 = GProperty(t1, obj1, relation_property);
                if (obj2) {
                    if (clear) WriteGProperty(t1, obj1, relation_property, 0);
                    else rfalse;
                }
            }
        }
    }
    rtrue;
];

Note that the above is from 6M62, but the 10.1 version seems logically identical.

The I7 code

if <relation> is empty...

and

now <relation> is empty;

will not work correctly when the right side of the relation is an enumerated kind of value (KOV) and the left side is either an object-descended type or a KOV. The type of failure will differ depending on the type of the left side.

Here is a demonstration of the effect on KOV <-> KOV relations:

"Bug Demonstration 1"

Place is a room.

A color is a kind of value. The colors are red, orange, yellow, green, blue, indigo and violet.

Counterpart relates one color to one color. The verb to counter means the counterpart relation.

When play begins:
	show relation the counterpart relation; [looks empty]
	if the counterpart relation is empty, say "Test 1: EMPTY."; [evals true -- OK]
	now red counters blue; [OK]
	now orange counters green; [OK]
	show relation the counterpart relation; [looks correct]
	if the counterpart relation is empty, say "Test 2: EMPTY."; [evals true-- incorrect]
	now the counterpart relation is empty; [no effect]
	show relation the counterpart relation; [previous assertions present]
	if the counterpart relation is empty, say "Test 3: EMPTY."; [evals true - incorrect]
	now yellow counters indigo; [OK]
	show relation the counterpart relation; [now three assertions]
	if the counterpart relation is empty, say "Test 4: EMPTY." [evals false - incorrect]

The failure to detect a non-empty relation and the failure to clear all assertions when the relation is made empty appear to be because the code is lacking initialization of local variable N2. This means that neither loop 2 nor loop 4 are ever entered. Based on the pattern seen in other routines, N2 should be set using the KOVDomainSize() routine before use as the gate condition for any loop. For example:

[ Relation_EmptyOtoO relation sym clear relation_property obj1 obj2 t1 t2 N1 N2;
    relation_property = RlnGetF(relation, RR_STORAGE);
    t1 = KindBaseTerm(RlnGetF(relation, RR_KIND), 0); ! Kind of left term
    t2 = KindBaseTerm(RlnGetF(relation, RR_KIND), 1); ! Kind of right term
	N2 = KOVDomainSize(t2);	! ADDED
	if (t2 == OBJECT_TY) {
	...
	} else {
		for (obj2=1: obj2<=N2: obj2++) {						! loop 2
		...
		}
	}
	if (t1 ~= t2) {
		if (t1 == OBJECT_TY) {
		...
		} else {
            for (obj1=1: obj1<=N2: obj1++) {					! loop 4
			...
			}
		}
	}
	rtrue;
];

If these changes are made, then the “Bug Demonstration 1” scenario will work as expected.

2 Likes

Here is a demonstration of the effect on object <-> KOV relations:

"Bug Demonstration 2"

Place is a room.

Alice is a woman in Place.

Bob is a man in Place.

Carl is a man in Place.

A color is a kind of value. The colors are red, orange, yellow, green, blue, indigo and violet.

Pairing relates one person to one color. The verb to pair with means the pairing relation.

When play begins:
	show relation the pairing relation; [correct display]
	if the pairing relation is empty, say "Test 1: EMPTY."; [<--- evaluates false, incorrect]
	now Alice pairs with red; [OK]
	now Bob pairs with orange; [OK]
	show relation the pairing relation; [correct display]
	if the pairing relation is empty, say "Test 2: EMPTY."; [evaluates false, OK]
	now the pairing relation is empty;
	show relation the pairing relation; [<--- corrupted display]
	if the pairing relation is empty, say "Test 3: EMPTY."; [evaluates true, should be false but failed empty]
	now Carl pairs with yellow; [<--- generates RTP]
	show relation the pairing relation; [<--- corrupted display]
	if the pairing relation is empty, say "Test 4: EMPTY." [<--- evaluates true but meaning unclear]

The initially empty relation is not evaluated correctly as empty. This is due to the third loop in the routine:

    if (t1 ~= t2) {											! <--- t1 is OBJECT_TY, t2 is KOV ID
        if (t1 == OBJECT_TY) {
            objectloop (obj1 provides relation_property) {  ! <--- only qualifying object is ValuePropertyHolder_* object
                obj2 = obj1.relation_property;				! <--- retrieves storage array address from ValuePropertyHolder_* object
                if (obj2) {									! <--- array address evaluates as true
                    if (clear) obj1.relation_property = nothing;
                    else rfalse;							! <--- returns false to indicate test failure
                }
            }

This loop appears to be intended to handle cases in which the relation assertions are stored as properties of objects, which is not the case in the example.

In addition to the above, note that an attempt to empty the relation results in severing of the connection between ValuePropertyHolder_* and the designated storage array held in .pNN_XXX_relation_storage as a result of the lines:

                if (obj2) {											! <--- array address evaluates as true
                    if (clear) obj1.relation_property = nothing;	! <--- clear flag set, so storage array address set to 0
                    else rfalse;
                }

As a result, the assertions of the relation are lost, and attempts to access them use inappropriate data. For example:

relation of people to colors: pairing relation
Pairing relates one person to one color:
  K2_thing  >=>  indigo

Note that this corrupted display is from 6M62; it did not occur when testing in 10.1.

Further, attempts to assert a relation in this state result in a run-time error:

[** Programming error: tried to write outside memory using --> **]

This RTE does occur in 10.1.

The result of the final test for emptiness (#4) is true in the example, but since the information needed to accurately determine the test result has been lost to the ValuePropertyHolder_* object, the result cannot mean what it is intended to.

1 Like

The overall logic of the function in selecting which loop to apply seems to not be properly matched to the I7 compiler’s choice of how to implement storage for the relation. This decision seems to be made based on the type of value held in the right side of the relation. If the right side is an object-descended type, then storage is implemented as a property attached to the kind in question. If the right side is an enumerated KOV type, then storage is implemented as an array linked to a special-purpose ValuePropertyHolder_* object.

When the routine is run, loops are activated depending on the combination of types:

LEFT	RIGHT	LOOPS
----	-----	---------------
obj		obj		1
obj		KOV		2, 3 [problems]
KOV		obj		1, 4 [problems]
KOV		KOV		2

Because the required handling is determined solely by the right side’s type, there does not seem to be a need to differentiate between cases in which the left and right sides are the same type and those in which they are different. It appears that the logic of the routine can be simplified to differentiate only on the type of the right side:

RIGHT	LOOPS
-----	---------------
obj		1
KOV		2

If the routine is replaced with this logic:

modified version
[ Relation_EmptyOtoO relation sym clear relation_property obj1 obj2 t1 t2 N1 N2; ! can remove N1 if desired
	relation_property = RlnGetF(relation, RR_STORAGE);
	t1 = KindBaseTerm(RlnGetF(relation, RR_KIND), 0); ! Kind of left term
	t2 = KindBaseTerm(RlnGetF(relation, RR_KIND), 1); ! Kind of right term
	if (t2 == OBJECT_TY) {
		objectloop (obj2 provides relation_property) {
			obj1 = obj2.relation_property;
			if (obj1) {
				if (clear) obj2.relation_property = nothing;
				else rfalse;
			}
		}
	} else {
		N2 = KOVDomainSize(t2);	! ADDED
		for (obj2=1: obj2<=N2: obj2++) {
			obj1 = GProperty(t2, obj2, relation_property);
			if (obj1) {
				if (clear) WriteGProperty(t2, obj2, relation_property, 0);
				else rfalse;
			}
		}
	}
	! loops 3 and 4 removed
	rtrue;
];

then all of the issues identified appear to be resolved.

Does anyone care to cross-check the above?

1 Like

Nice catch, Otis. I’ll check it out and file a bug as appropriate later.

1 Like

If the official code is to be changed, it might be also be of interest to look at converting some hardcoded array index values to the constant intended to hold that hardcoded value (VTOVS_LEFT_DOMAIN_SIZE).

The first is Relation_EmptyVtoV() (third and fourth loops):

Relation_EmptyVtoV
[ Relation_EmptyVtoV relation sym clear vtov_structure obj1 obj2 pr pr2 proutine1 proutine2;
	vtov_structure = RlnGetF(relation, RR_STORAGE);
	pr = vtov_structure-->VTOVS_LEFT_INDEX_PROP;
	pr2 = vtov_structure-->VTOVS_RIGHT_INDEX_PROP;
	proutine1 = vtov_structure-->VTOVS_LEFT_PRINTING_ROUTINE;
	proutine2 = vtov_structure-->VTOVS_RIGHT_PRINTING_ROUTINE;
 
	if (pr && pr2) {
		objectloop (obj1 provides pr)
			objectloop (obj2 provides pr2) {
				if (sym && obj2 > obj1) continue;
				if (Relation_TestVtoV(obj1, relation, obj2)) {
					if (clear) Relation_NowNVtoV(obj1, relation, obj2, sym);
					else rfalse;
				}
			}
		return;
	}
	if (pr && (pr2==0)) {
		objectloop (obj1 provides pr)
			for (obj2=1:obj2<=vtov_structure-->VTOVS_RIGHT_DOMAIN_SIZE:obj2++) {
				if (Relation_TestVtoV(obj1, relation, obj2)) {
					if (clear) Relation_NowNVtoV(obj1, relation, obj2, sym);
					else rfalse;
				}
			}
		return;
	}
	if ((pr==0) && (pr2)) {
		for (obj1=1:obj1<=vtov_structure-->2:obj1++)	! <--- change to obj1<=vtov_structure-->VTOVS_LEFT_DOMAIN_SIZE?
			objectloop (obj2 provides pr2) {
				if (Relation_TestVtoV(obj1, relation, obj2)) {
					if (clear) Relation_NowNVtoV(obj1, relation, obj2, sym);
					else rfalse;
				}
			}
		return;
	}
	for (obj1=1:obj1<=vtov_structure-->2:obj1++)		! <--- change to obj1<=vtov_structure-->VTOVS_LEFT_DOMAIN_SIZE?
		for (obj2=1:obj2<=vtov_structure-->VTOVS_RIGHT_DOMAIN_SIZE:obj2++)
			if (Relation_TestVtoV(obj1, relation, obj2)) {
				if (Relation_TestVtoV(obj1, relation, obj2)) {
					if (clear) Relation_NowNVtoV(obj1, relation, obj2, sym);
					else rfalse;
				}
		}
	rtrue;
];

The second is Relation_ShowVtoV() (third and fourth loops):

Relation_ShowVtoV
[ Relation_ShowVtoV relation sym x obj1 obj2 pr pr2 proutine1 proutine2 vtov_structure;
	vtov_structure = RlnGetF(relation, RR_STORAGE);
	pr = vtov_structure-->VTOVS_LEFT_INDEX_PROP;
	pr2 = vtov_structure-->VTOVS_RIGHT_INDEX_PROP;
	proutine1 = vtov_structure-->VTOVS_LEFT_PRINTING_ROUTINE;
	proutine2 = vtov_structure-->VTOVS_RIGHT_PRINTING_ROUTINE;
 
	if (pr && pr2) {
		objectloop (obj1 provides pr)
		  objectloop (obj2 provides pr2) {
				if (sym && obj2 > obj1) continue;
				if (Relation_TestVtoV(obj1, relation, obj2)) {
					if (x == 0) { print (string) RlnGetF(relation, RR_DESCRIPTION), ":^"; x=1; }
					print "  ", (The) obj1;
					if (sym) print "  <=>  "; else print "  >=>  ";
					print (the) obj2, "^";
				}
		  }
		return;
	}
	if (pr && (pr2==0)) {
		objectloop (obj1 provides pr)
		  for (obj2=1:obj2<=vtov_structure-->VTOVS_RIGHT_DOMAIN_SIZE:obj2++) {
				if (Relation_TestVtoV(obj1, relation, obj2)) {
					if (x == 0) { print (string) RlnGetF(relation, RR_DESCRIPTION), ":^"; x=1; }
					print "  ", (The) obj1, "  >=>  ";
					(proutine2).call(obj2);
					print "^";
				}
		  }
		return;
	}
	if ((pr==0) && (pr2)) {
		for (obj1=1:obj1<=vtov_structure-->2:obj1++)	! <--- change to obj1<=vtov_structure-->VTOVS_LEFT_DOMAIN_SIZE?
		  objectloop (obj2 provides pr2) {
				if (Relation_TestVtoV(obj1, relation, obj2)) {
					if (x == 0) { print (string) RlnGetF(relation, RR_DESCRIPTION), ":^"; x=1; }
					print "  ";
					(proutine1).call(obj1);
					print "  >=>  ", (the) obj2, "^";
				}
		  }
		return;
	}
	for (obj1=1:obj1<=vtov_structure-->2:obj1++)		! <--- change to obj1<=vtov_structure-->VTOVS_LEFT_DOMAIN_SIZE?
		  for (obj2=1:obj2<=vtov_structure-->VTOVS_RIGHT_DOMAIN_SIZE:obj2++)
			if (Relation_TestVtoV(obj1, relation, obj2)) {
				if (x == 0) { print (string) RlnGetF(relation, RR_DESCRIPTION), ":^"; x=1; }
				print "  ";
				(proutine1).call(obj1);
				print "  >=>  ";
				(proutine2).call(obj2);
				print "^";
		  }
];

The third is FastVtoVRelRouteTo() (initialization):

[ FastVtoVRelRouteTo relation from to count
	    domainsize cache cache2 left_ix ox oy oj offset axy axj ayj;
	    domainsize = RlnGetF(relation, RR_STORAGE)-->2; ! Number of left instances  ! <--- HERE
	    left_ix = RlnGetF(relation, RR_STORAGE)-->VTOVS_LEFT_INDEX_PROP;
		...
2 Likes

I got way behind on my crucial “report the incredibly specific bugs Otis finds” duties, but this is now
I7-2376.

1 Like