Skip to content

fix comparison op JVP returning bool tangents instead of input dtype#3253

Merged
angeloskath merged 2 commits into
ml-explore:mainfrom
mm65x:fix-jvp-comparison-bool-tangents
Mar 16, 2026
Merged

fix comparison op JVP returning bool tangents instead of input dtype#3253
angeloskath merged 2 commits into
ml-explore:mainfrom
mm65x:fix-jvp-comparison-bool-tangents

Conversation

@mm65x

@mm65x mm65x commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

Proposed changes

#3081

the six comparison op JVP implementations (Equal, Greater, GreaterEqual,
Less, LessEqual, NotEqual) return zeros(shape, bool_, stream()),
producing bool-typed tangents. this causes downstream ops like negative to
crash with [negative] Not supported for bool, use logical_not instead.

the fix changes them to zeros(shape, tangents[0].dtype(), stream()) so the
zero tangent preserves the input tangent's dtype. this matches the pattern
used by LogicalNot::jvp which already uses zeros_like(tangents[0]).

Checklist

  • I have read the CONTRIBUTING document
  • I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the necessary documentation (if needed)

@angeloskath angeloskath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good thanks!

@mm65x mm65x force-pushed the fix-jvp-comparison-bool-tangents branch from 908ba09 to 685628f Compare March 16, 2026 10:37
@mm65x

mm65x commented Mar 16, 2026

Copy link
Copy Markdown
Contributor Author

fixed the failing test, was calling an invalid op on the primal path.

@angeloskath angeloskath merged commit 505fc98 into ml-explore:main Mar 16, 2026
16 checks passed
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.

2 participants