Skip to content

Commit ffcd755

Browse files
kapellirohithzcbenz
authored andcommitted
Fix Log and Equal is_equivalent ignoring primitive state
Log carries base_ and Equal carries equal_nan_, but both used DEFINE_DEFAULT_IS_EQUIVALENT(), so the compile simplify pass merged log/log2/log10 of the same input, and array_equal with and without equal_nan, into a single node.
1 parent dcf4b2a commit ffcd755

4 files changed

Lines changed: 66 additions & 2 deletions

File tree

mlx/primitives.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1941,6 +1941,11 @@ std::pair<std::vector<array>, std::vector<int>> Equal::vmap(
19411941
return {{equal(a, b, stream())}, {to_ax}};
19421942
}
19431943

1944+
bool Equal::is_equivalent(const Primitive& other) const {
1945+
const Equal& e_other = static_cast<const Equal&>(other);
1946+
return equal_nan_ == e_other.equal_nan_;
1947+
}
1948+
19441949
std::vector<array> Equal::vjp(
19451950
const std::vector<array>& primals,
19461951
const std::vector<array>& cotangents,
@@ -2795,6 +2800,11 @@ std::pair<std::vector<array>, std::vector<int>> Log::vmap(
27952800
axes};
27962801
}
27972802

2803+
bool Log::is_equivalent(const Primitive& other) const {
2804+
const Log& l_other = static_cast<const Log&>(other);
2805+
return base_ == l_other.base_;
2806+
}
2807+
27982808
std::vector<array> Log1p::vjp(
27992809
const std::vector<array>& primals,
28002810
const std::vector<array>& cotangents,

mlx/primitives.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -975,8 +975,8 @@ class Equal : public UnaryPrimitive {
975975

976976
DEFINE_VMAP()
977977
DEFINE_GRADS()
978-
DEFINE_DEFAULT_IS_EQUIVALENT()
979978
DEFINE_INPUT_OUTPUT_SHAPE()
979+
bool is_equivalent(const Primitive& other) const override;
980980

981981
const char* name() const override {
982982
if (equal_nan_) {
@@ -1325,8 +1325,8 @@ class Log : public UnaryPrimitive {
13251325

13261326
DEFINE_VMAP()
13271327
DEFINE_GRADS()
1328-
DEFINE_DEFAULT_IS_EQUIVALENT()
13291328
DEFINE_INPUT_OUTPUT_SHAPE()
1329+
bool is_equivalent(const Primitive& other) const override;
13301330

13311331
Base state() const {
13321332
return base_;

python/tests/test_compile.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,6 +1623,29 @@ def test_compile_abs_unsigned(self):
16231623
x = mx.array([1, 2, 3], dtype)
16241624
self.assertTrue(mx.array_equal(mx.compile(fun)(x), fun(x)))
16251625

1626+
def test_compile_different_log_bases(self):
1627+
# The logs are intermediates, since outputs are not simplified.
1628+
def entropies(p):
1629+
nats = -mx.sum(p * mx.log(p))
1630+
bits = -mx.sum(p * mx.log2(p))
1631+
return mx.stack([nats, bits])
1632+
1633+
p = np.array([0.1, 0.2, 0.3, 0.4], dtype=np.float32)
1634+
expected = np.array(
1635+
[-(p * np.log(p)).sum(), -(p * np.log2(p)).sum()], dtype=np.float32
1636+
)
1637+
out = mx.compile(entropies)(mx.array(p))
1638+
self.assertTrue(np.allclose(out, expected, atol=1e-5))
1639+
1640+
def test_compile_equal_nan(self):
1641+
def fun(x):
1642+
return mx.stack(
1643+
[mx.array_equal(x, x), mx.array_equal(x, x, equal_nan=True)]
1644+
)
1645+
1646+
x = mx.array([1.0, float("nan"), 3.0])
1647+
self.assertTrue(mx.array_equal(mx.compile(fun)(x), mx.array([False, True])))
1648+
16261649

16271650
if __name__ == "__main__":
16281651
mlx_tests.MLXTestRunner()

tests/compile_tests.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,37 @@ TEST_CASE("test no simplify") {
212212
set_compile_mode(CompileMode::enabled);
213213
}
214214

215+
auto log_bases(const std::vector<array>& inputs) {
216+
auto a = inputs[0];
217+
return std::vector<array>{log(a) + log2(a)};
218+
};
219+
220+
auto equal_nan_variants(const std::vector<array>& inputs) {
221+
auto a = inputs[0];
222+
return std::vector<array>{
223+
stack({array_equal(a, a), array_equal(a, a, true)})};
224+
};
225+
226+
TEST_CASE("test no simplify different primitive state") {
227+
set_compile_mode(CompileMode::no_fuse);
228+
auto a = array({2.0f, 8.0f});
229+
auto b = compile(log_bases)({a})[0];
230+
CHECK(b.inputs()[0].id() != b.inputs()[1].id());
231+
CHECK(allclose(b, log(a) + log2(a)).item<bool>());
232+
233+
auto c = array({1.0f, std::numeric_limits<float>::quiet_NaN()});
234+
auto d = compile(equal_nan_variants)({c})[0];
235+
CHECK(array_equal(d, array({false, true})).item<bool>());
236+
237+
// Matching state still simplifies.
238+
auto same_base = [](const std::vector<array>& inputs) -> std::vector<array> {
239+
return {log(inputs[0]) + log(inputs[0])};
240+
};
241+
auto e = compile(same_base)({a})[0];
242+
CHECK(e.inputs()[0].id() == e.inputs()[1].id());
243+
set_compile_mode(CompileMode::enabled);
244+
}
245+
215246
auto multi_one(const std::vector<array>&) {
216247
auto a = array(1.0);
217248
auto b = array(2.0);

0 commit comments

Comments
 (0)