Skip to content

Commit fc5dec3

Browse files
committed
Compare proven representation differences without relaxing exact values
Keep exact wide-integer comparisons, preserve raw evidence, and identify nullable entity-reference improvement explicitly. Independent review and 30 comparator tests pass; all124 completed local/file/S3 candidate cells compare successfully. Remove obsolete engine skips: all4 affected public native-route tests pass. Signed-off-by: Adi Wadaskar <awadaskar@nvidia.com>
1 parent ecd862d commit fc5dec3

2 files changed

Lines changed: 155 additions & 6 deletions

File tree

‎packages/structured-data-models-client/tests/test_both_routes_describe_themselves_alike.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ def envelope(scored: list[int], known: list[int]) -> dict:
8787

8888

8989
def test_the_two_routes_report_the_same_halves_for_the_same_question():
90-
pytest.importorskip('sdm_relational_engine')
9190

9291
schema = one_table()
9392
query = 'PREDICT t.amount FOR EACH t.rid'
@@ -111,7 +110,6 @@ def test_the_two_routes_report_the_same_halves_for_the_same_question():
111110
@pytest.mark.parametrize('model', ['tabular', 'relational'])
112111
def test_a_request_agrees_with_its_own_counts(model):
113112
"""The halves and the counts are two readings of one thing."""
114-
pytest.importorskip('sdm_relational_engine')
115113

116114
chosen = KumoTabular() if model == 'tabular' else KumoRelational()
117115
with warnings.catch_warnings():
@@ -126,7 +124,6 @@ def test_a_request_agrees_with_its_own_counts(model):
126124

127125

128126
def test_a_relational_request_shows_the_tables_it_carries():
129-
pytest.importorskip('sdm_relational_engine')
130127

131128
users = pd.DataFrame({'uid': range(300), 'seg': RNG.choice([*'xy'], 300)})
132129
events = pd.DataFrame(

‎tests/parity/compare.py‎

Lines changed: 155 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import json
88
import math
99
import operator
10+
import re
1011
from pathlib import Path
1112

1213
from identity import IdentityError, canonical
@@ -29,6 +30,8 @@ def difference(left, right, tolerance, path='$'):
2930
return None
3031
if isinstance(left, int) and isinstance(right, int):
3132
return path
33+
if tolerance['absolute'] == 0 and tolerance['relative'] == 0:
34+
return path
3235
return (
3336
None
3437
if math.isclose(
@@ -62,6 +65,126 @@ def difference(left, right, tolerance, path='$'):
6265
return None if left == right else path
6366

6467

68+
def equivalent_reader_time_units(old, new):
69+
"""Normalize only exact timestamp values with compatible declared resolutions."""
70+
exact = {'absolute': 0, 'relative': 0}
71+
if difference(old['data'], new['data'], exact) is not None:
72+
return
73+
if len(old['schema']) != len(new['schema']):
74+
return
75+
for before, after in zip(old['schema'], new['schema'], strict=True):
76+
if before[0] != after[0] or before[1] == after[1]:
77+
continue
78+
a = re.fullmatch(r'timestamp\[(s|ms|us|ns)(, tz=[^]]+)?\]', before[1])
79+
b = re.fullmatch(r'timestamp\[(s|ms|us|ns)(, tz=[^]]+)?\]', after[1])
80+
if not a or not b or a[2] != b[2]:
81+
continue
82+
digits = min(
83+
{'s': 0, 'ms': 3, 'us': 6, 'ns': 9}[a[1]],
84+
{'s': 0, 'ms': 3, 'us': 6, 'ns': 9}[b[1]],
85+
)
86+
at = old['data']['columns'].index(before[0])
87+
for row in old['data']['rows']:
88+
value = row[at]
89+
if value is None:
90+
continue
91+
match = (
92+
re.fullmatch(
93+
r'[+-]?\d{4,}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(?:\.(\d{1,9}))?(?:Z|[+-]\d{2}:\d{2})?',
94+
value,
95+
)
96+
if isinstance(value, str)
97+
else None
98+
)
99+
if not match or any(c != '0' for c in (match[1] or '')[digits:]):
100+
break
101+
else:
102+
# Caller compares all other schema fields and every original value.
103+
after[1] = before[1]
104+
105+
106+
def equivalent_derived_entity_refs(old_payloads, new_payloads, fixture):
107+
"""Accept exact nullable integer storage only for declared temporal-link references."""
108+
if (
109+
not isinstance(old_payloads, list)
110+
or not isinstance(new_payloads, list)
111+
or len(old_payloads) != len(new_payloads)
112+
):
113+
return False
114+
improved = False
115+
for old, new in zip(old_payloads, new_payloads, strict=True):
116+
task = old.get('task', {})
117+
if (
118+
task != new.get('task')
119+
or task.get('kind') != 'temporal_link_prediction'
120+
):
121+
continue
122+
entities = task.get('entity_table_names', [])
123+
old_schema, new_schema = old.get('schema', {}), new.get('schema', {})
124+
links = old_schema.get('relationships', [])
125+
if links != new_schema.get('relationships', []):
126+
continue
127+
a = old_schema.get('instance_table', {}).get('columns', {})
128+
b = new_schema.get('instance_table', {}).get('columns', {})
129+
for index, target in enumerate(entities):
130+
name = f'__kumo_entity_ref_{index}'
131+
before, after = a.get(name, {}), b.get(name, {})
132+
if (
133+
before.get('dtype') != 'float64'
134+
or after.get('dtype') != 'int64'
135+
):
136+
continue
137+
if (
138+
before.get('stype') != 'ID'
139+
or {**before, 'dtype': 'int64'} != after
140+
):
141+
continue
142+
matching = [
143+
link
144+
for link in links
145+
if not link.get('source_table')
146+
and link.get('source_columns') == [name]
147+
]
148+
if (
149+
len(matching) != 1
150+
or matching[0].get('target_table') != target
151+
or matching[0].get('target_columns')
152+
!= [fixture['primary_keys'].get(target)]
153+
):
154+
continue
155+
valid = True
156+
observed = False
157+
for half in ('context', 'predict'):
158+
left = old.get(half, {}).get('instance_table', {})
159+
right = new.get(half, {}).get('instance_table', {})
160+
if (
161+
left.get('columns') != right.get('columns')
162+
or len(left.get('rows', [])) != len(right.get('rows', []))
163+
or name not in left.get('columns', [])
164+
):
165+
valid = False
166+
break
167+
at = left['columns'].index(name)
168+
for row, other in zip(left['rows'], right['rows'], strict=True):
169+
x, y = row[at], other[at]
170+
if x is None and y is None:
171+
continue
172+
observed = True
173+
if (
174+
type(x) is not float
175+
or type(y) is not int
176+
or not math.isfinite(x)
177+
or not x.is_integer()
178+
or int(x) != y
179+
):
180+
valid = False
181+
break
182+
if valid and observed:
183+
before['dtype'] = 'int64'
184+
improved = True
185+
return improved
186+
187+
65188
def assess(
66189
data, baseline, candidate, *, live=True, selected=None, candidate_commit
67190
):
@@ -165,10 +288,24 @@ def assess(
165288
copy.deepcopy(old.get('semantic')),
166289
copy.deepcopy(new.get('semantic')),
167290
)
291+
improved_entity_ref = False
292+
if cell[0] == 'relational.temporal-link' and wanted == 'passed':
293+
from contract import HERE
294+
295+
fixture = json.loads((HERE / case['fixture']).read_text())
296+
improved_entity_ref = equivalent_derived_entity_refs(
297+
old_semantic['request'], new_semantic['request'], fixture
298+
)
168299
if (
169300
cell[2] == 'relational'
170301
and case.get('comparison') == 'fixture-source-identity'
171302
and wanted == 'passed'
303+
and difference(
304+
old_semantic['request'],
305+
new_semantic['request'],
306+
{'absolute': 0, 'relative': 0},
307+
)
308+
is not None
172309
):
173310
from contract import HERE
174311

@@ -209,22 +346,37 @@ def readers(record, fixture=fixture):
209346
data = copy.deepcopy(reading['data'])
210347
at = data['columns'].index(fixture['primary_keys'][name])
211348
data['rows'].sort(key=operator.itemgetter(at))
212-
values[name] = {'schema': reading['schema'], 'data': data}
349+
values[name] = {
350+
'schema': copy.deepcopy(reading['schema']),
351+
'data': data,
352+
}
213353
return values
214354

355+
old_readers, new_readers = readers(old), readers(new)
356+
for name in old_readers.keys() & new_readers.keys():
357+
equivalent_reader_time_units(old_readers[name], new_readers[name])
215358
changed = changed or difference(
216-
readers(old), readers(new), exact, '$.reader'
359+
old_readers, new_readers, exact, '$.reader'
217360
)
218361

219362
row.update(
220363
status='failed' if changed else 'passed',
221-
verdict=expected.get('verdict', 'Parity never supported')
364+
verdict=(
365+
'Improvement (private bridge)'
366+
if (new.get('endpoint') or {}).get('transport')
367+
== 'private-bridge'
368+
else 'Improvement'
369+
)
370+
if improved_entity_ref and not changed
371+
else expected.get('verdict', 'Parity never supported')
222372
if expected['status'] == 'refused'
223373
else 'Parity (private bridge)'
224374
if (new.get('endpoint') or {}).get('transport') == 'private-bridge'
225375
else 'Parity',
226376
reason=f'difference at {changed}'
227377
if changed
378+
else 'Exact nullable integer storage for declared temporal entity reference'
379+
if improved_entity_ref
228380
else 'matching evidence',
229381
)
230382
scoped = [

0 commit comments

Comments
 (0)