Rename autodiff L_op and R_op methods to pull_back and push_forward by ricardoV94 · Pull Request #1983 · pymc-devs/pytensor

ricardoV94 · 2026-03-17T14:28:49Z

Closes #182

L_op -> pull_back
R_op -> push_forward
push_forward: takes outputs as well (consistent signature with L_op)
push_forward: Uses DisconnectType instead of None (used to be the same in L_op, but they changed after a while)

Claude generated

ricardoV94 · 2026-03-18T12:47:26Z

Did you ever see this one @jessegrabowski

FAILED tests/tensor/test_slinalg.py::TestSchur::test_schur_empty - ValueError: negative dimensions not allowed
Apply node that caused the error: Schur{output='real', overwrite_a=False, sort=None}(<Matrix(float64, shape=(?, ?))>)
Toposort index: 0
Inputs types: [TensorType(float64, shape=(None, None))]
Inputs shapes: [(0, 0)]
Inputs strides: [(0, 0)]
Inputs values: [array([], shape=(0, 0), dtype=float64)]
Outputs clients: [[output[0](Schur{output='real', overwrite_a=False, sort=None}.0)], [output[1](Schur{output='real', overwrite_a=False, sort=None}.1)]]

Backtrace when the node is created (use PyTensor flag traceback__limit=N to make it longer):
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_callers.py", line 121, in _multicall
    res = hook_impl.function(*args)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/_pytest/runner.py", line 179, in pytest_runtest_call
    item.runtest()
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/_pytest/python.py", line 1720, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_hooks.py", line 512, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_callers.py", line 121, in _multicall
    res = hook_impl.function(*args)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/_pytest/python.py", line 166, in pytest_pyfunc_call
    result = testfunction(**testargs)
  File "/home/runner/work/pytensor/pytensor/tests/tensor/test_slinalg.py", line 1363, in test_schur_empty
    T, Z = schur(A)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_callers.py", line 121, in _multicall
    res = hook_impl.function(*args)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/_pytest/runner.py", line 179, in pytest_runtest_call
    item.runtest()
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/_pytest/python.py", line 1720, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_hooks.py", line 512, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/pluggy/_callers.py", line 121, in _multicall
    res = hook_impl.function(*args)
  File "/home/runner/micromamba/envs/pytensor-test/lib/python3.14/site-packages/_pytest/python.py", line 166, in pytest_pyfunc_call
    result = testfunction(**testargs)
  File "/home/runner/work/pytensor/pytensor/tests/tensor/test_slinalg.py", line 1363, in test_schur_empty
    T, Z = schur(A)

HINT: Use the PyTensor flag `exception_verbosity=high` for a debug print-out and storage map footprint of this Apply node.
= 1 failed, 664 passed, 29 skipped, 22 xfailed, 55 warnings in 1363.94s (0:22:43) =
 ** On entry to DGEES  parameter number  6 had an illegal value

jessegrabowski · 2026-03-18T12:50:16Z

No, but looks like an overly strict check somewhere? Like its doing if any(dim <= 0) to check for negativity. I can try to dig in and see if its an upstream version issue but it'll take me a second to get to.

ricardoV94 · 2026-03-18T13:15:23Z

Maybe a new version of scipy thing?

ricardoV94 · 2026-03-18T13:16:06Z

I haven't seen it fail in other CI. Does it have random numbers in it?

Armavica · 2026-03-26T16:27:25Z

You are sometimes adding/renaming a fourth argument with different names: eval_points, output_grads, g_outs: it sometimes looks like a missed opportunity to directly call them co|tangents as in the function prototypes!
If you don't feel like regenerating the PR I'm happy to clean it up afterwards (it is part of what I was doing in #1792 before the renaming)

ricardoV94 · 2026-03-26T19:33:52Z

You are sometimes adding/renaming a fourth argument with different names: eval_points, output_grads, g_outs: it sometimes looks like a missed opportunity to directly call them co|tangents as in the function prototypes!
If you don't feel like regenerating the PR I'm happy to clean it up afterwards (it is part of what I was doing in #1792 before the renaming)

If this sounds good to go otherwise then I would merge and gladly take your cleanup after

jessegrabowski · 2026-04-01T03:24:26Z

~~wanna depreciate the grad method while we're at it?~~

I'm 0/2 tonight on commenting before I read the code

jessegrabowski · 2026-04-01T03:25:11Z

pytensor/link/numba/dispatch/typed_list.py

        def all_equal(x, y):
            return (
                x.shape == y.shape
+                and len(x.data) == len(y.data)


split this to a separate PR?

jessegrabowski · 2026-04-01T03:29:22Z

pytensor/graph/op.py

+
+        Parameters
+        ----------
+        inputs


typehints are missing

jessegrabowski · 2026-04-01T03:29:41Z

pytensor/graph/op.py

+
+        Returns
+        -------
+        list of Variable


nit: always name the return variable in the docstring even if the variable is never actually created

jessegrabowski · 2026-04-01T03:30:51Z

pytensor/graph/op.py

+
+        References
+        ----------
+        .. [1] Giles, Mike. 2008. "An Extended Collection of Matrix Derivative


Is this really the best reference for explaining what is a VJP? This is a autodiff cookbook paper

jessegrabowski · 2026-04-01T03:31:28Z

pytensor/graph/op.py

+        -------
+        list of Variable
+            The tangent vectors w.r.t. each output. One `Variable` per output.
+


poor pushforward doesn't even get a reference :(

jessegrabowski · 2026-04-01T03:31:40Z

pytensor/graph/op.py

+        `R_op` method, `push_forward` must never use ``None`` to indicate
+        disconnected outputs.
+
+        Parameters


same type/naming complaints

jessegrabowski · 2026-04-01T03:32:55Z

pytensor/gradient.py

                try:
                    y = x.type.filter_variable(y)
                except TypeError:
-                    # This is a hack


are we blessing this hack now?

jessegrabowski · 2026-04-01T03:38:38Z

pytensor/compile/builders.py

                )
            warnings.warn(
-                "grad_overrides is deprecated in favor of lop_overrides. Using it will lead to an error in the future.",
+                "grad_overrides is deprecated in favor of pull_back_overrides.",


This warning has already been here for a while right? We could just remove this one.

jessegrabowski · 2026-04-01T03:39:58Z

pytensor/compile/builders.py

-                self._lop_op_cache[disconnected_output_grads] = lop_overrides
-                lop_overrides.kwargs["on_unused_input"] = "ignore"
-                return lop_overrides
+        if isinstance(pb_overrides, OpFromGraph):


nit: i don't love the use of pb/pf in this PR. I'd prefer we just write the words out to be as clear as possible. This code is conceptual enough as it is, and people have wide screens.

ricardoV94 force-pushed the autodiff_cleanup branch from 680c62d to dfacf05 Compare March 18, 2026 09:01

ricardoV94 added major gradients labels Mar 18, 2026

ricardoV94 changed the title ~~autodiff cleanup~~ Rename autodiff L_op and R_op methods to pull_back and push_forward Mar 18, 2026

ricardoV94 force-pushed the autodiff_cleanup branch 3 times, most recently from 08e6493 to a7b636a Compare March 18, 2026 10:29

ricardoV94 closed this Mar 18, 2026

ricardoV94 reopened this Mar 18, 2026

ricardoV94 marked this pull request as ready for review March 18, 2026 13:28

ricardoV94 force-pushed the autodiff_cleanup branch 2 times, most recently from a7200ac to 2409f4a Compare March 20, 2026 22:36

ricardoV94 requested review from Armavica, aseyboldt and jessegrabowski March 20, 2026 23:03

Armavica mentioned this pull request Mar 25, 2026

Typing #1792

Open

11 tasks

jessegrabowski approved these changes Apr 1, 2026

View reviewed changes

ricardoV94 added 4 commits April 1, 2026 22:50

Fix sparse random equality in numba

4fcb012

Rename autodiff methods

7205c75

Stop using None for disconnected gradients in push_forward

f07f4da

Use new methods internally

4b4c419

ricardoV94 force-pushed the autodiff_cleanup branch from 2409f4a to 4b4c419 Compare April 1, 2026 20:51

Conversation

ricardoV94 commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ricardoV94 commented Mar 18, 2026

Uh oh!

jessegrabowski commented Mar 18, 2026

Uh oh!

ricardoV94 commented Mar 18, 2026

Uh oh!

ricardoV94 commented Mar 18, 2026

Uh oh!

Armavica commented Mar 26, 2026

Uh oh!

ricardoV94 commented Mar 26, 2026

Uh oh!

jessegrabowski commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ricardoV94 commented Mar 17, 2026 •

edited

Loading

jessegrabowski commented Apr 1, 2026 •

edited

Loading