gemini-code-assist[bot] commented on code in PR #19584:
URL: https://github.com/apache/tvm/pull/19584#discussion_r3258223293


##########
tests/python/arith/test_arith_transitive_comparison.py:
##########
@@ -0,0 +1,170 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+"""Tests for TransitiveComparisonAnalyzer and the per-key index."""
+
+import pytest
+
+import tvm
+import tvm.ir
+import tvm.testing
+from tvm import tirx
+from tvm.script import tirx as T
+
+
+def test_single_bind_provability():
+    analyzer = tvm.arith.Analyzer()
+    x = tirx.Var("x", "int32")
+    analyzer.bind(x, tvm.ir.Range.from_min_extent(0, 100))
+    assert analyzer.can_prove(x >= 0)
+    assert analyzer.can_prove(x < 100)
+    assert analyzer.can_prove(x <= 99)
+    assert not analyzer.can_prove(x >= 1)
+
+
+def test_many_binds_correctness_preserved():
+    analyzer = tvm.arith.Analyzer()
+    vars_ = [tirx.Var(f"v{i}", "int32") for i in range(2048)]
+    for i, v in enumerate(vars_):
+        analyzer.bind(v, tvm.ir.Range.from_min_extent(i, 10))
+    for i in (0, len(vars_) // 2, len(vars_) - 1):
+        v = vars_[i]
+        assert analyzer.can_prove(v >= i)
+        assert analyzer.can_prove(v < i + 10)
+        assert not analyzer.can_prove(v >= i + 1)
+
+
+def test_bind_override_clears_old_constraints():
+    analyzer = tvm.arith.Analyzer()
+    x = tirx.Var("x", "int32")
+    analyzer.bind(x, tvm.ir.Range.from_min_extent(0, 100))
+    assert analyzer.can_prove(x < 100)
+    analyzer.bind(x, tvm.ir.Range.from_min_extent(50, 10), allow_override=True)
+    assert analyzer.can_prove(x >= 50)
+    assert analyzer.can_prove(x < 60)
+    assert analyzer.can_prove(x >= 0)
+    assert analyzer.can_prove(x < 100)

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The assertions `assert analyzer.can_prove(x >= 0)` and `assert 
analyzer.can_prove(x < 100)` do not effectively verify that the old constraints 
were cleared, because the new range `[50, 60)` is a subset of the old range 
`[0, 100)`. To properly test that the override clears previous constraints, 
consider using a new range that does not satisfy the old constraints.
   
   ```suggestion
       analyzer.bind(x, tvm.ir.Range.from_min_extent(200, 10), 
allow_override=True)
       assert analyzer.can_prove(x >= 200)
       assert analyzer.can_prove(x < 210)
       assert not analyzer.can_prove(x < 100)
   ```



##########
tests/python/arith/test_arith_transitive_comparison.py:
##########
@@ -0,0 +1,170 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+"""Tests for TransitiveComparisonAnalyzer and the per-key index."""
+
+import pytest
+
+import tvm
+import tvm.ir
+import tvm.testing
+from tvm import tirx
+from tvm.script import tirx as T
+
+
+def test_single_bind_provability():
+    analyzer = tvm.arith.Analyzer()
+    x = tirx.Var("x", "int32")
+    analyzer.bind(x, tvm.ir.Range.from_min_extent(0, 100))
+    assert analyzer.can_prove(x >= 0)
+    assert analyzer.can_prove(x < 100)
+    assert analyzer.can_prove(x <= 99)
+    assert not analyzer.can_prove(x >= 1)
+
+
+def test_many_binds_correctness_preserved():
+    analyzer = tvm.arith.Analyzer()
+    vars_ = [tirx.Var(f"v{i}", "int32") for i in range(2048)]
+    for i, v in enumerate(vars_):
+        analyzer.bind(v, tvm.ir.Range.from_min_extent(i, 10))
+    for i in (0, len(vars_) // 2, len(vars_) - 1):
+        v = vars_[i]
+        assert analyzer.can_prove(v >= i)
+        assert analyzer.can_prove(v < i + 10)
+        assert not analyzer.can_prove(v >= i + 1)
+
+
+def test_bind_override_clears_old_constraints():
+    analyzer = tvm.arith.Analyzer()
+    x = tirx.Var("x", "int32")
+    analyzer.bind(x, tvm.ir.Range.from_min_extent(0, 100))
+    assert analyzer.can_prove(x < 100)
+    analyzer.bind(x, tvm.ir.Range.from_min_extent(50, 10), allow_override=True)
+    assert analyzer.can_prove(x >= 50)
+    assert analyzer.can_prove(x < 60)
+    assert analyzer.can_prove(x >= 0)
+    assert analyzer.can_prove(x < 100)
+
+
+def test_scoped_constraint_enter_and_exit():
+    analyzer = tvm.arith.Analyzer()
+    x = tirx.Var("x", "int32")
+    y = tirx.Var("y", "int32")
+    analyzer.bind(x, tvm.ir.Range.from_min_extent(0, 100))
+    with analyzer.constraint_scope(y < x):
+        assert analyzer.can_prove(y < x)
+    assert not analyzer.can_prove(y < x)
+
+
+def test_cross_key_lookup():
+    # `b > a` is stored with `b` on lhs; query `a < b` has `a` on lhs.
+    # The dual-key index must still resolve it via WithLHS.

Review Comment:
   ![medium](https://www.gstatic.com/codereviewagent/medium-priority.svg)
   
   The comment mentions the "dual-key index", but `constraint_scope` adds to 
`scoped_knowns_`, which is a flat vector and not part of the `knowns_by_key_` 
index (as noted in the PR description). To test the index specifically, 
consider using `analyzer.bind` with an expression that places the variable of 
interest on the RHS of the resulting `Comparison` (e.g., `analyzer.bind(x, y)` 
and then querying `y == x`).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to