Skip to content

Restore atan_difference gradients at negative-one products - #2039

Open
DiadkoShmek wants to merge 1 commit into
tensorflow:mainfrom
DiadkoShmek:lezo50-atan-difference-pole-gradients-20261105
Open

DiadkoShmek wants to merge 1 commit into
tensorflow:mainfrom
DiadkoShmek:lezo50-atan-difference-pole-gradients-20261105

Conversation

@DiadkoShmek

Copy link
Copy Markdown

atan_difference has the correct value at x*y == -1, but its constant selected branch removes the smooth partial derivatives while the inactive singular quotient poisons reverse AD. Both TensorFlow and JAX are affected. This reaches an ordinary finite likelihood:

import tensorflow as tf
import tensorflow_probability as tfp

params = tf.Variable([0., 1.], dtype=tf.float64)  # loc, scale
with tf.GradientTape() as tape:
    lp = tfp.distributions.TruncatedCauchy(
        params[0], params[1], low=-1., high=1.).log_prob(.25)
print(tape.gradient(lp, params))  # currently [NaN, NaN]

The correct score is [8/17, -15/17 + 2/pi], approximately [0.470588, -0.245733]. The density value itself is already correct; forward AD also loses the normalizer's derivative.

Keep the cancellation-resistant quotient away from the pole. At opposite-sign poles, atan(x) - atan(y) is cancellation-free and supplies the correct derivatives. Use the existing native conditional to evaluate that additional work only when a pole is present. The inactive quotient denominator is made finite as well, because JAX vmap may lower conditionals to selects.

Validation:

  • Focused regression tests: TensorFlow 2.21 CPU 30 passes, JAX/JAXlib 0.7 CPU 14 passes, versus 20 failures/24 passing controls on BASE. Native regressions cover both dtypes, broadcast adjoints and the public likelihood. The final test also exercises actual TensorFlow ForwardAccumulator through tf.function in four TF2 contexts; its focused BASE comparison fails all four and the candidate passes. Reverse checks remain in all regimes; the TF1 forward limitation is guarded.
  • Separate analytic/finite-difference comparisons pass 184 TensorFlow and 184 JAX checks. Independent review passes an unchanged 380-check oracle (108 failures on BASE), plus 72 low/high-scale and mixed/nested-vmap controls. Additional 160 JAX and 60 TensorFlow control comparisons pass; ten NumPy value controls match BASE, without a NumPy AD claim.
  • All three affected files pass the unmodified project Pylint configuration with actual Pylint 4.1.3; three existing configuration UserWarnings remain. Seven pre-existing redundant-parenthesis assignments in the already changed owner file are normalized for the whole-file lint gate; those syntax-only changes are AST-equivalent.

Two fixed, warmed, non-pole JAX CPU value-and-gradient fixtures measure scalar: 10.8455 → 11.823 µs (+0.9775µs/+9.0%) and batch of 4,096: 152.184 → 153.5765 µs (+0.9%). These are bounded local cost controls, with compilation recorded separately; they do not establish a general speed or no-regression guarantee.

The numerical scope is finite real inputs and first AD on the tested backends. Full project/Bazel/nightly, GPU, sharding, higher AD and universal IEEE behavior were not tested. The separate Lambert-W change in #2038 is not included.

I directed this work and take responsibility for the contribution and its follow-up. AI assistance was used in producing the change.

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