Skip to content

Pass the caller stream through complex l2_norm and scatter max/min VJP - #4818

Open
Augustas11 wants to merge 1 commit into
ml-explore:mainfrom
Augustas11:stream-complex-norm-scatter-vjp
Open

Augustas11 wants to merge 1 commit into
ml-explore:mainfrom
Augustas11:stream-complex-norm-scatter-vjp

Conversation

@Augustas11

Copy link
Copy Markdown
  • I understand it is strictly prohibited to use AI to write PR description

AI usage disclosure: drafted with AI assistance from a stream audit. Both call sites were checked against multiply and equal in mlx/ops.h, which take a stream. The test suite has not been run locally.

Follow-up to #4746. Two call sites still drop the stream the caller passed, and the stream checker misses them because they are operator overloads.

Complex l2_norm did abs(a, s) * abs(a, s). operator* takes no stream, so the product is built on the thread's default stream. Real inputs already go through square(a, s). The complex path now uses multiply(abs(a, s), abs(a, s), s).

Max and min Scatter::vjp did gathered_result == updates inside a multiply that already received stream(). operator== takes no stream. That comparison is now equal(gathered_result, updates, stream()).

This only changes complex l2_norm and the max/min scatter gradient.

The existing cross-thread test in tests/scheduler_tests.cpp now builds both graphs on a thread-unsafe stream and evaluates them from the other thread: a complex linalg::norm, and a scatter_max VJP.

Augustas from Malibu

operator* and operator== build on the default stream. Complex l2_norm
and the max/min scatter gradient both went through them.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant