Skip to content

Keep is_self_adjoint hint in LinearOperatorScaledIdentity.inverse() - #129948

Merged
copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
b84330808:fix-scaled-identity-inverse-self-adjoint
Nov 9, 2026
Merged

copybara-service[bot] merged 1 commit into
tensorflow:masterfrom
b84330808:fix-scaled-identity-inverse-self-adjoint

Conversation

@b84330808

Copy link
Copy Markdown
Contributor

LinearOperatorScaledIdentity._linop_inverse always sets is_self_adjoint=True. That holds for a real multiplier, but not for a complex one, and adjoint() returns self when the hint is True, so the adjoint of the inverse comes out wrong:

import tensorflow as tf
op = tf.linalg.LinearOperatorScaledIdentity(num_rows=2, multiplier=1j)
inv = op.inverse()                     # -1j * I
inv.is_self_adjoint                    # True
inv.adjoint().to_dense()               # [[-1j, 0], [0, -1j]]
tf.linalg.adjoint(inv.to_dense())      # [[ 1j, 0], [0,  1j]]

The same wrong self is used by inv.H, inv.matmul(other_operator, adjoint=True) and inv.solve(other_operator, adjoint=True), and inv.assert_self_adjoint() fails.

The fix passes self.is_self_adjoint, like _linop_adjoint and the other operators' _linop_inverse already do. For a real multiplier nothing changes: the constructor sets is_self_adjoint=True on its own.

test_scaled_identity_inverse_complex_multiplier compares inverse().adjoint() with the dense adjoint for multiplier 1j, and checks that a real multiplier still gives a self-adjoint inverse. It fails before the fix (graph and eager) and passes after. The whole linear_operator_identity_test.py (2689 tests), linear_operator_diag_test.py and linear_operator_addition_test.py pass with the change. I ran them against tf-nightly 2.23.0.dev20261103 with the patched module, not a source build.

The inverse of a scaled identity with a complex multiplier was always marked self-adjoint, so inverse().adjoint() returned the inverse itself: for multiplier 1j it gave -1j * I instead of 1j * I. Pass the operator's own hint instead; real multipliers are still auto-set to self-adjoint by the constructor.
@google-ml-butler google-ml-butler Bot added the size:S CL Change Size: Small label Nov 4, 2026
@google-ml-butler
google-ml-butler Bot requested a review from cantonios November 4, 2026 04:28
@google-ml-butler google-ml-butler Bot added the awaiting review Pull request awaiting review label Nov 4, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes a bug in LinearOperatorScaledIdentity._linop_inverse where the inverse operator was always marked as self-adjoint. It now correctly propagates the is_self_adjoint property of the original operator, which is necessary when dealing with complex multipliers. A corresponding unit test test_scaled_identity_inverse_complex_multiplier has been added to verify this behavior for both complex and real multipliers. I have no feedback to provide as the changes are correct and well-tested.

@keerthanakadiri
keerthanakadiri requested a review from a team November 4, 2026 04:38
@keerthanakadiri keerthanakadiri self-assigned this Nov 4, 2026
@github-project-automation github-project-automation Bot moved this to Assigned Reviewer in PR Queue Nov 4, 2026
@google-ml-butler google-ml-butler Bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Nov 5, 2026
@github-project-automation github-project-automation Bot moved this from Assigned Reviewer to Approved by Reviewer in PR Queue Nov 5, 2026
@nithyak0204 nithyak0204 removed the awaiting review Pull request awaiting review label Nov 6, 2026
@copybara-service
copybara-service Bot merged commit eba0084 into tensorflow:master Nov 9, 2026
23 checks passed
@github-project-automation github-project-automation Bot moved this from Approved by Reviewer to Merged in PR Queue Nov 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kokoro:force-run Tests on submitted change ready to pull PR ready for merge process size:S CL Change Size: Small

Projects

Status: Merged

Development

Successfully merging this pull request may close these issues.

4 participants